Skip to content

[no-release-notes] fix tests dropping null checks on not nullable columns #2839

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
jycor wants to merge 6 commits into
base: main
Choose a base branch
from
Open

[no-release-notes] fix tests dropping null checks on not nullable columns #2839

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
f968c5c
bump and fix tests
Jun 10, 2026
6f250ed
prevent overflow in index
Jun 11, 2026
3e63bdc
bump
Jun 11, 2026
6ea696d
uncomment
Jun 11, 2026
1be39de
asdf
Jun 11, 2026
dcb6518
remove todo
Jun 11, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions go.mod
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,10 @@ require (
github.com/PuerkitoBio/goquery v1.8.1
github.com/cockroachdb/apd/v3 v3.2.3
github.com/cockroachdb/errors v1.7.5
github.com/dolthub/dolt/go v0.40.5-0.20260609180312-20cac47c5cf9
github.com/dolthub/dolt/go v0.40.5-0.20260611201749-2de22d2c05da
github.com/dolthub/eventsapi_schema v0.0.0-20260310172945-37a9265ade69
github.com/dolthub/flatbuffers/v23 v23.3.3-dh.2
github.com/dolthub/go-mysql-server v0.20.1-0.20260608231025-6579b6af9de2
github.com/dolthub/go-mysql-server v0.20.1-0.20260610222239-82ca9a426101
github.com/dolthub/pg_query_go/v6 v6.0.0-20251215122834-fb20be4254d1
github.com/dolthub/sqllogictest/go v0.0.0-20240618184124-ca47f9354216
github.com/dolthub/vitess v0.0.0-20260604210335-0893abc80542
Expand Down Expand Up @@ -105,7 +105,7 @@ require (
github.com/dolthub/aws-sdk-go-ini-parser v0.0.0-20250305001723-2821c37f6c12 // indirect
github.com/dolthub/dolt-mcp v0.3.4 // indirect
github.com/dolthub/fslock v0.0.5 // indirect
github.com/dolthub/go-icu-regex v0.0.0-20260412212219-49724d547866 // indirect
github.com/dolthub/go-icu-regex v0.0.0-20260610153742-72563bc7ca83 // indirect
github.com/dolthub/gozstd v0.0.0-20240423170813-23a2903bca63 // indirect
github.com/dolthub/ishell v0.0.0-20260414231531-5f031e3e9037 // indirect
github.com/dolthub/jsonpath v0.0.2-0.20240227200619-19675ab05c71 // indirect
Expand Down
12 changes: 6 additions & 6 deletions go.sum
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -246,18 +246,18 @@ github.com/dolthub/aws-sdk-go-ini-parser v0.0.0-20250305001723-2821c37f6c12 h1:I
github.com/dolthub/aws-sdk-go-ini-parser v0.0.0-20250305001723-2821c37f6c12/go.mod h1:rN7X8BHwkjPcfMQQ2QTAq/xM3leUSGLfb+1Js7Y6TVo=
github.com/dolthub/dolt-mcp v0.3.4 h1:AyG5cw+fNWXDHXujtQnqUPZrpWtPg6FN6yYtjv1pP44=
github.com/dolthub/dolt-mcp v0.3.4/go.mod h1:bCZ7KHvDYs+M0e+ySgmGiNvLhcwsN7bbf5YCyillLrk=
github.com/dolthub/dolt/go v0.40.5-0.20260609180312-20cac47c5cf9 h1:tDoj5+v3WjAT6Ospy44shBeaOhrbTcEcs6tNRaB4zxw=
github.com/dolthub/dolt/go v0.40.5-0.20260609180312-20cac47c5cf9/go.mod h1:qUI6QdFuKRbg4/7U+gNrm1dcuH3ZpCSJNgT6GnZ2fTQ=
github.com/dolthub/dolt/go v0.40.5-0.20260611201749-2de22d2c05da h1:zYDtpzy7/HFxOzr8KHGhBtKfiYmx6g8yYZCgslIPuys=
github.com/dolthub/dolt/go v0.40.5-0.20260611201749-2de22d2c05da/go.mod h1:6K1zrTpiNo4fyzyZA71LXHCrgt1tWY8QC6aRoCe9WlA=
github.com/dolthub/eventsapi_schema v0.0.0-20260310172945-37a9265ade69 h1:JShhbqMw26nKx3pqqu/cFxOpzBkN+4elVhzuUfgDw2k=
github.com/dolthub/eventsapi_schema v0.0.0-20260310172945-37a9265ade69/go.mod h1:SSLraQS/jGLYFgff3vuZ+JbVUct6vyEeMzjLBqWqoyM=
github.com/dolthub/flatbuffers/v23 v23.3.3-dh.2 h1:u3PMzfF8RkKd3lB9pZ2bfn0qEG+1Gms9599cr0REMww=
github.com/dolthub/flatbuffers/v23 v23.3.3-dh.2/go.mod h1:mIEZOHnFx4ZMQeawhw9rhsj+0zwQj7adVsnBX7t+eKY=
github.com/dolthub/fslock v0.0.5 h1:QoXhBhgY1oumHE26qyE7tgmXUT8qjJwxsIzo54O/B/k=
github.com/dolthub/fslock v0.0.5/go.mod h1:sdofYYqE0D79zNZyB4/kmlnsQOVap1C2yByjGKSirEM=
github.com/dolthub/go-icu-regex v0.0.0-20260412212219-49724d547866 h1:U6gSf5I0e6h6GP1/5Sa7D2lWW1CWfcVPtY5wkyHq6jY=
github.com/dolthub/go-icu-regex v0.0.0-20260412212219-49724d547866/go.mod h1:F3cnm+vMRK1HaU6+rNqQrOCyR03HHhR1GWG2gnPOqaE=
github.com/dolthub/go-mysql-server v0.20.1-0.20260608231025-6579b6af9de2 h1:xrRFEiifbxFFpRvNVswPFYXxg6UKgaWUW7UmFI+Z7NU=
github.com/dolthub/go-mysql-server v0.20.1-0.20260608231025-6579b6af9de2/go.mod h1:A3nEC4RgBm9UzuQCGhQhJDQAiFMPsZU3NPKbGhEd8R4=
github.com/dolthub/go-icu-regex v0.0.0-20260610153742-72563bc7ca83 h1:FEMjCGEroDnY/BXyAffVZxUpXhP2GpoUJyyq5KaLn8c=
github.com/dolthub/go-icu-regex v0.0.0-20260610153742-72563bc7ca83/go.mod h1:F3cnm+vMRK1HaU6+rNqQrOCyR03HHhR1GWG2gnPOqaE=
github.com/dolthub/go-mysql-server v0.20.1-0.20260610222239-82ca9a426101 h1:Abci4YqSTb0jRO7H/BGljkSJ0hkAZvZlim74JLAWros=
github.com/dolthub/go-mysql-server v0.20.1-0.20260610222239-82ca9a426101/go.mod h1:xrwqyZnkLeqNm/LNp57NB+otApMXv1UJohn9dSkbgR0=
github.com/dolthub/gozstd v0.0.0-20240423170813-23a2903bca63 h1:OAsXLAPL4du6tfbBgK0xXHZkOlos63RdKYS3Sgw/dfI=
github.com/dolthub/gozstd v0.0.0-20240423170813-23a2903bca63/go.mod h1:lV7lUeuDhH5thVGDCKXbatwKy2KW80L4rMT46n+Y2/Q=
github.com/dolthub/ishell v0.0.0-20260414231531-5f031e3e9037 h1:oIW9HwuWrhxv+4HZxA+QQSKHLqWFyXZ2FmNjUYwkdiM=
Expand Down
4 changes: 2 additions & 2 deletions server/tables/pgcatalog/pg_constraint.go
View file
Open in desktop

@itoqa itoqa Bot Jun 11, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

View replay

Medium severity Range conversion drops valid constraint rows

What failed: The query returns no rows when six constraint names are expected, and explain output shows open-lower-bound formatting changes alongside the semantic drift.

Impact · Steps · Stub / mock · Analysis · Why this is likely a bug
  • Severity: Medium Medium severity
  • Impact: Constraint metadata queries can miss valid rows on this path, which can break expected introspection behavior for affected workloads. The issue is persistent but scoped to a specific index-range conversion flow.
  • Steps to Reproduce:
    1. Build and run the repository with the upgraded dependency set from this PR.
    2. Execute go test ./testing/go -run TestPgConstraintIndexes --count=1.
    3. Inspect the assertion for SELECT conname FROM pg_catalog.pg_constraint WHERE (conname LIKE '%_pkey' OR conname LIKE '%_key') AND connamespace = 2200 ORDER BY conname.
    4. Observe that no rows are returned where six constraint names are expected.
  • Stub / mock content: Real SCRAM sign-in was bypassed by turning off the auth gate in server/authentication_scram.go so the regression failure could be reproduced consistently in the local QA environment.
  • Code Analysis: I inspected server/tables/pgcatalog/pg_constraint.go and verified that the pg_constraint_conname_nsp_index upper-bound logic increments schema upper bounds while leaving conNameUpper empty, then uses that as the less-than key. With the same file's (conname, schema) comparator ordering, this upper key can sort below normal names and exclude valid rows.
  • Why this is likely a bug: The production range-bound construction and comparator order together provide a concrete code path that explains the observed empty-result regression for a valid query.
Relevant code

server/tables/pgcatalog/pg_constraint.go:344-354

if conNameRange.HasUpperBound() || schemaOidRange.HasUpperBound() {
	// our less-than upper bound depends on whether we have a prefix match or both fields were set
	if !schemaOidUpperSet {
		conNameUpper = fmt.Sprintf("%s%o", conNameUpper, rune(0))
	} else {
		schemaOidUpper = schemaOidUpper + 1
	}
	lt = &pgConstraint{
		name:            conNameUpper,
		schemaOidNative: schemaOidUpper,
	}
}

server/tables/pgcatalog/pg_constraint.go:578-583

func lessConstraintNameSchema(a, b []*pgConstraint) bool {
	if a[0].name == b[0].name {
		return a[0].schemaOidNative < b[0].schemaOidNative
	}
	return a[0].name < b[0].name
}
Copy prompt for an agent 
Ito QA identified the following failure during automated PR testing. Please investigate and propose a fix.

**Medium severity — Range conversion drops valid constraint rows**

**What failed:** The query returns no rows when six constraint names are expected, and explain output shows open-lower-bound formatting changes alongside the semantic drift.

- **Impact:** Constraint metadata queries can miss valid rows on this path, which can break expected introspection behavior for affected workloads. The issue is persistent but scoped to a specific index-range conversion flow.
- **Steps to reproduce:**
  1. Build and run the repository with the upgraded dependency set from this PR.
  2. Execute `go test ./testing/go -run TestPgConstraintIndexes --count=1`.
  3. Inspect the assertion for `SELECT conname FROM pg_catalog.pg_constraint WHERE (conname LIKE '%_pkey' OR conname LIKE '%_key') AND connamespace = 2200 ORDER BY conname`.
  4. Observe that no rows are returned where six constraint names are expected.
- **Stub / mock content:** Real SCRAM sign-in was bypassed by turning off the auth gate in `server/authentication_scram.go` so the regression failure could be reproduced consistently in the local QA environment.
- **Code analysis:** I inspected `server/tables/pgcatalog/pg_constraint.go` and verified that the `pg_constraint_conname_nsp_index` upper-bound logic increments schema upper bounds while leaving `conNameUpper` empty, then uses that as the less-than key. With the same file's `(conname, schema)` comparator ordering, this upper key can sort below normal names and exclude valid rows.
- **Why this is likely a bug:** The production range-bound construction and comparator order together provide a concrete code path that explains the observed empty-result regression for a valid query.

**Relevant code:**

`server/tables/pgcatalog/pg_constraint.go:344-354`

~~~go
if conNameRange.HasUpperBound() || schemaOidRange.HasUpperBound() {
	// our less-than upper bound depends on whether we have a prefix match or both fields were set
	if !schemaOidUpperSet {
		conNameUpper = fmt.Sprintf("%s%o", conNameUpper, rune(0))
	} else {
		schemaOidUpper = schemaOidUpper + 1
	}
	lt = &pgConstraint{
		name:            conNameUpper,
		schemaOidNative: schemaOidUpper,
	}
}
~~~

`server/tables/pgcatalog/pg_constraint.go:578-583`

~~~go
func lessConstraintNameSchema(a, b []*pgConstraint) bool {
	if a[0].name == b[0].name {
		return a[0].schemaOidNative < b[0].schemaOidNative
	}
	return a[0].name < b[0].name
}
~~~

Original file line number Diff line number Diff line change
Expand Up @@ -278,11 +278,11 @@ func (p PgConstraintHandler) getIndexScanRange(rng sql.Range, index sql.Index) (
if typOidUpperSet {
if nameUpperSet {
nameUpper = fmt.Sprintf("%s%o", nameUpper, rune(0))
} else {
} else if typOidUpper < math.MaxUint32 { // prevent overflow
typOidUpper = typOidUpper + 1
}
} else {
if !relOidUpperSet {
if !relOidUpperSet && relOidUpper < math.MaxUint32 { // prevent overflow
relOidUpper = relOidUpper + 1
}
}
Expand Down
10 changes: 5 additions & 5 deletions testing/go/index_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -198,7 +198,7 @@ func TestBasicIndexing(t *testing.T) {
{"Sort(test.pk ASC)"},
{" └─ IndexedTableAccess(test)"},
{" ├─ index: [test.pk,test.v1]"},
{" ├─ filters: [{[NULL, ∞), (NULL, f)}, {[NULL, ∞), (t, ∞)}]"},
{" ├─ filters: [{(NULL, ∞), (NULL, f)}, {(NULL, ∞), (t, ∞)}]"},
{" └─ columns: [pk v1]"},
},
},
Expand Down Expand Up @@ -288,7 +288,7 @@ func TestBasicIndexing(t *testing.T) {
{" ├─ (test.v1 = jointable.v3 AND test.v2 = 22)"},
{" ├─ IndexedTableAccess(test)"},
{" │ ├─ index: [test.pk]"},
{" │ ├─ filters: [{[NULL, ∞)}]"},
{" │ ├─ filters: [{(NULL, ∞)}]"},
{" │ └─ columns: [pk v1 v2]"},
{" └─ Table"},
{" ├─ name: jointable"},
Expand All @@ -308,7 +308,7 @@ func TestBasicIndexing(t *testing.T) {
{" ├─ (test.v1 = jointable.v3 AND test.v2 = jointable.v4)"},
{" ├─ IndexedTableAccess(test)"},
{" │ ├─ index: [test.pk]"},
{" │ ├─ filters: [{[NULL, ∞)}]"},
{" │ ├─ filters: [{(NULL, ∞)}]"},
{" │ └─ columns: [pk v1 v2]"},
{" └─ Table"},
{" ├─ name: jointable"},
Expand Down Expand Up @@ -462,7 +462,7 @@ func TestBasicIndexing(t *testing.T) {
{" ├─ (test.v1 = jointable.v3 AND test.v2 = 22)"},
{" ├─ IndexedTableAccess(test)"},
{" │ ├─ index: [test.pk]"},
{" │ ├─ filters: [{[NULL, ∞)}]"},
{" │ ├─ filters: [{(NULL, ∞)}]"},
{" │ └─ columns: [pk v1 v2]"},
{" └─ Table"},
{" ├─ name: jointable"},
Expand Down Expand Up @@ -496,7 +496,7 @@ func TestBasicIndexing(t *testing.T) {
{" ├─ (test.v1 = sq.v3 AND test.v2 = sq.v4)"},
{" ├─ IndexedTableAccess(test)"},
{" │ ├─ index: [test.pk]"},
{" │ ├─ filters: [{[NULL, ∞)}]"},
{" │ ├─ filters: [{(NULL, ∞)}]"},
{" │ └─ columns: [pk v1 v2]"},
{" └─ TableAlias(sq)"},
{" └─ Table"},
Expand Down
Loading