test insert rules - #3017
Closed
jycor wants to merge 1 commit into
Closed
Conversation
Contributor
|
|
SummaryThe run covers core data-entry behavior across successful inserts, generated and default values, bound inputs, returned results, conflict handling, validation errors, and atomicity under failed batches and retries. It also checks persistence across connections and runtime readiness, while exposing deployment-toolchain compatibility issues in source-based container builds. Safe to merge — the application behavior exercised by this PR passed without regressions or PR-attributable failures. The Docker source-build failures are unrelated pre-existing toolchain issues and should be flagged for follow-up rather than treated as merge blockers. Tests run by Ito
Additional Findings DetailsThese findings are unrelated to the current changes but were observed during testing. 🟠 Clean Docker builds cannot resolve dependencies
Evidence Package🟡 Docker source build cannot create the binary
Evidence PackageTip Reply with @itoqa to send us feedback on this test run. |
Contributor
|
Commit: SummaryCoverage spans normal and prepared inserts, returned and default-generated values, bulk loading, conflict handling, and edge cases involving invalid data and partition boundaries. Happy-path behavior generally works, but partitioned-table validation and routing show serious integrity failures under invalid or boundary inputs. Not safe to merge yet — this PR is associated with high-severity partitioning failures that accept invalid values and leave parent and child data inconsistent, creating a risk of silent data corruption. Separate conflict-update and bulk-load validation issues are also present but are unrelated to this PR and should be treated as follow-up caveats. Tests run by Ito
Additional Findings DetailsThese findings are unrelated to the current changes but were observed during testing. 🟠 Conflict updates reject incoming row values
Evidence Package🟡 Oversized text is silently shortened
Evidence PackageTip Reply with @itoqa to send us feedback on this test run. |
![itoqa[bot]](https://avatars.githubusercontent.com/in/2622614?s=60&v=4){kind=small}
| github.com/dolthub/eventsapi_schema v0.0.0-20260715220557-d9b4a1c6b4d4 | ||
| github.com/dolthub/flatbuffers/v23 v23.3.3-dh.2 | ||
| github.com/dolthub/go-mysql-server v0.20.1-0.20260803203407-e49664d6062c | ||
| github.com/dolthub/go-mysql-server v0.20.1-0.20260803232249-e787bab6e784 |
There was a problem hiding this comment.
Invalid rows enter wrong partitions
What failed: The valid rows were accepted, but the invalid inserts also returned success. The first child stored b=11, the second child stayed empty, and the parent count did not match the expected partition routing; invalid rows should have been rejected without being committed.
Impact · Steps · Stub / mock · Analysis · Why this is likely a bug
- Severity: High
- Impact: Users can insert rows outside a partition's allowed range, and those rows may be stored in the wrong partition. This can silently corrupt partitioned data and produce incorrect query results.
- Steps to Reproduce:
- Create a range-partitioned table with a child covering ('a', 1) through ('a', 10) and a second child covering ('a', 10) through ('a', 20).
- Insert one valid row directly into the first child and one valid row through the parent table.
- Insert ('a', 11) directly into the first child and try out-of-range values through the parent table.
- Query both child tables and the parent table, then compare the stored rows and counts with the insert results.
- Stub / mock content: The test used a local-only partition fixture and equivalent ATTACH PARTITION syntax because the original PARTITION OF and CASCADE setup syntax was unsupported; no mocks or route interception were used.
- Code Analysis: The regression fixture at testing/go/regression/tests/insert.sql:110-133 explicitly requires direct child inserts outside a child's range to fail, including INSERT INTO part1 VALUES ('a', 11), and requires only in-range values to succeed. The test plan at testing/go/regression/tests/insert.sql:195-203 also expects parent inserts outside the configured ranges to be rejected. In the local application, server/ast/insert.go:61-75 resolves the target table, server/ast/insert.go:76-91 converts the columns and source rows, and server/ast/insert.go:109-123 constructs the vitess INSERT passed to the execution engine; this path does not implement an alternate partition router or bound validator that could explain the observed behavior. The runtime accepted all four probes and persisted b=11 in child1 while child2 remained empty, which is consistent with the pinned execution engine failing to enforce the partition metadata. The PR diff changes go.mod line 12 from go-mysql-server pseudo-version e49664d6062c to e787bab6e784, and changes go.sum to match. The smallest practical fix is to correct or revert that dependency version to one that enforces partition bounds and routes rows correctly, or apply the upstream partition-bound fix before updating the pin; do not accept successful INSERT results until the target child and range constraint have been validated.
- Why this is likely a bug: This is not only a failed assertion or an unsupported fixture: the equivalent local ATTACH PARTITION setup succeeded, valid inserts worked, and the same execution accepted an out-of-range row and stored it in a child whose declared range excludes it. That creates silently incorrect partitioned data and contradicts the repository's explicit PostgreSQL regression expectations. The AST conversion preserves the requested target and values, so the evidence points to the execution-engine version selected by the PR rather than browser rendering or a test-only patch. A targeted dependency correction or upstream partition-enforcement fix addresses the observed defect without requiring a broad rewrite.
Relevant code
testing/go/regression/tests/insert.sql:110-133
-- direct partition inserts should check partition bound constraint
create table range_parted (
a text,
b int
) partition by range (a, (b+0));
create table part1 partition of range_parted for values from ('a', 1) to ('a', 10);
create table part2 partition of range_parted for values from ('a', 10) to ('a', 20);
-- fail
insert into part1 values ('a', 11);
insert into part1 values ('b', 1);
-- ok
insert into part1 values ('a', 1);server/ast/insert.go:61-123
switch node := node.Table.(type) {
case *tree.TableName:
tableName, err = nodeTableName(ctx, node)
...
rows, err = nodeSelect(ctx, node.Rows)
...
return &vitess.Insert{
Action: vitess.InsertStr,
Table: tableName,
Columns: columns,
Rows: rows,
}, nilgo.mod:12
github.com/dolthub/go-mysql-server v0.20.1-0.20260803232249-e787bab6e784Evidence Package
Copy prompt for an agent
Ito QA identified the following failure during automated PR testing. Please investigate and propose a fix.
**High severity — Invalid rows enter wrong partitions**
**What failed:** The valid rows were accepted, but the invalid inserts also returned success. The first child stored b=11, the second child stayed empty, and the parent count did not match the expected partition routing; invalid rows should have been rejected without being committed.
- **Impact:** Users can insert rows outside a partition's allowed range, and those rows may be stored in the wrong partition. This can silently corrupt partitioned data and produce incorrect query results.
- **Steps to reproduce:**
1. Create a range-partitioned table with a child covering ('a', 1) through ('a', 10) and a second child covering ('a', 10) through ('a', 20).
2. Insert one valid row directly into the first child and one valid row through the parent table.
3. Insert ('a', 11) directly into the first child and try out-of-range values through the parent table.
4. Query both child tables and the parent table, then compare the stored rows and counts with the insert results.
- **Stub / mock content:** The test used a local-only partition fixture and equivalent ATTACH PARTITION syntax because the original PARTITION OF and CASCADE setup syntax was unsupported; no mocks or route interception were used.
- **Code analysis:** The regression fixture at testing/go/regression/tests/insert.sql:110-133 explicitly requires direct child inserts outside a child's range to fail, including INSERT INTO part1 VALUES ('a', 11), and requires only in-range values to succeed. The test plan at testing/go/regression/tests/insert.sql:195-203 also expects parent inserts outside the configured ranges to be rejected. In the local application, server/ast/insert.go:61-75 resolves the target table, server/ast/insert.go:76-91 converts the columns and source rows, and server/ast/insert.go:109-123 constructs the vitess INSERT passed to the execution engine; this path does not implement an alternate partition router or bound validator that could explain the observed behavior. The runtime accepted all four probes and persisted b=11 in child1 while child2 remained empty, which is consistent with the pinned execution engine failing to enforce the partition metadata. The PR diff changes go.mod line 12 from go-mysql-server pseudo-version e49664d6062c to e787bab6e784, and changes go.sum to match. The smallest practical fix is to correct or revert that dependency version to one that enforces partition bounds and routes rows correctly, or apply the upstream partition-bound fix before updating the pin; do not accept successful INSERT results until the target child and range constraint have been validated.
- **Why this is likely a bug:** This is not only a failed assertion or an unsupported fixture: the equivalent local ATTACH PARTITION setup succeeded, valid inserts worked, and the same execution accepted an out-of-range row and stored it in a child whose declared range excludes it. That creates silently incorrect partitioned data and contradicts the repository's explicit PostgreSQL regression expectations. The AST conversion preserves the requested target and values, so the evidence points to the execution-engine version selected by the PR rather than browser rendering or a test-only patch. A targeted dependency correction or upstream partition-enforcement fix addresses the observed defect without requiring a broad rewrite.
**Relevant code:**
`testing/go/regression/tests/insert.sql:110-133`
~~~sql
-- direct partition inserts should check partition bound constraint
create table range_parted (
a text,
b int
) partition by range (a, (b+0));
create table part1 partition of range_parted for values from ('a', 1) to ('a', 10);
create table part2 partition of range_parted for values from ('a', 10) to ('a', 20);
-- fail
insert into part1 values ('a', 11);
insert into part1 values ('b', 1);
-- ok
insert into part1 values ('a', 1);
~~~
`server/ast/insert.go:61-123`
~~~go
switch node := node.Table.(type) {
case *tree.TableName:
tableName, err = nodeTableName(ctx, node)
...
rows, err = nodeSelect(ctx, node.Rows)
...
return &vitess.Insert{
Action: vitess.InsertStr,
Table: tableName,
Columns: columns,
Rows: rows,
}, nil
~~~
`go.mod:12`
~~~mod
github.com/dolthub/go-mysql-server v0.20.1-0.20260803232249-e787bab6e784
~~~
| github.com/dolthub/eventsapi_schema v0.0.0-20260715220557-d9b4a1c6b4d4 | ||
| github.com/dolthub/flatbuffers/v23 v23.3.3-dh.2 | ||
| github.com/dolthub/go-mysql-server v0.20.1-0.20260803203407-e49664d6062c | ||
| github.com/dolthub/go-mysql-server v0.20.1-0.20260803232249-e787bab6e784 |
There was a problem hiding this comment.
Invalid partition values are accepted
What failed: The boundary inserts returned success, the parent reported three rows, and both attached child tables reported zero rows. The out-of-range value k=20 was accepted, while the CHECK constraint error appeared only during cleanup instead of rejecting the insert atomically.
Impact · Steps · Stub / mock · Analysis · Why this is likely a bug
- Severity: High
- Impact: Partitioned tables can accept invalid inserts and leave rows or counts out of sync. This can silently corrupt stored data and make later queries return incomplete or incorrect results.
- Steps to Reproduce:
- Create a partitioned parent table with adjacent child ranges and a constraint-bearing column.
- Insert valid boundary values and an out-of-range value through the parent table.
- Query the parent and each child table and compare the reported insert counts with the stored rows.
- Check whether the invalid insert fails immediately and leaves the transaction and table contents unchanged.
- Stub / mock content: The test used a local Doltgres instance and an equivalent ALTER TABLE ATTACH PARTITION setup because the original PARTITION OF syntax was unsupported; no application behavior was mocked or bypassed.
- Code Analysis: The regression fixture in testing/go/regression/tests/insert.sql:195-221 defines the expected partition behavior: values inside adjacent ranges must route to the proper child, an out-of-range value must fail, and no inconsistent row or count may remain. The local INSERT translator in server/ast/insert.go:61-123 resolves the target table, converts the input rows, and passes the original target and rows into vitess.Insert; it contains no alternative routing or constraint bypass that could explain the mismatch. Therefore the observed acceptance and parent/child divergence occur in the execution engine selected by the module graph. This PR changes go.mod:12 from go-mysql-server e49664d6062c to e787bab6e784, and go.sum records the matching checksum, so the changed dependency selection is a direct practical cause of the behavior under test. The smallest fix is to revert or correct that go-mysql-server version until partition routing validates the child bounds before reporting success and preserves the insert atomically.
- Why this is likely a bug: A successful INSERT must correspond to a row in the correct partition, and an out-of-range partition key must be rejected before it changes visible state. The recorded result violates both guarantees: the parent count increased without matching child rows, an invalid key was accepted, and constraint failure was deferred until cleanup. This is a production execution defect rather than a browser artifact because the SQL output directly records the command results and the source shows that Doltgres forwards the INSERT to the pinned engine. The dependency pin changed in this PR, so reverting that pin is a targeted mitigation; the engine should otherwise validate routing and child constraints before returning INSERT success.
Relevant code
testing/go/regression/tests/insert.sql:195-221
-- Check tuple routing for partitioned tables
-- fail
insert into range_parted values ('a', 0);
-- ok
insert into range_parted values ('a', 1);
insert into range_parted values ('a', 10);
-- fail
insert into range_parted values ('a', 20);
select tableoid::regclass, * from range_parted;server/ast/insert.go:61-123
tableName, err = nodeTableName(ctx, node)
rows, err = nodeSelect(ctx, node.Rows)
return &vitess.Insert{
Table: tableName,
Columns: columns,
Rows: rows,
}, nilgo.mod:12
github.com/dolthub/go-mysql-server v0.20.1-0.20260803232249-e787bab6e784Evidence Package
Copy prompt for an agent
Ito QA identified the following failure during automated PR testing. Please investigate and propose a fix.
**High severity — Invalid partition values are accepted**
**What failed:** The boundary inserts returned success, the parent reported three rows, and both attached child tables reported zero rows. The out-of-range value k=20 was accepted, while the CHECK constraint error appeared only during cleanup instead of rejecting the insert atomically.
- **Impact:** Partitioned tables can accept invalid inserts and leave rows or counts out of sync. This can silently corrupt stored data and make later queries return incomplete or incorrect results.
- **Steps to reproduce:**
1. Create a partitioned parent table with adjacent child ranges and a constraint-bearing column.
2. Insert valid boundary values and an out-of-range value through the parent table.
3. Query the parent and each child table and compare the reported insert counts with the stored rows.
4. Check whether the invalid insert fails immediately and leaves the transaction and table contents unchanged.
- **Stub / mock content:** The test used a local Doltgres instance and an equivalent ALTER TABLE ATTACH PARTITION setup because the original PARTITION OF syntax was unsupported; no application behavior was mocked or bypassed.
- **Code analysis:** The regression fixture in testing/go/regression/tests/insert.sql:195-221 defines the expected partition behavior: values inside adjacent ranges must route to the proper child, an out-of-range value must fail, and no inconsistent row or count may remain. The local INSERT translator in server/ast/insert.go:61-123 resolves the target table, converts the input rows, and passes the original target and rows into vitess.Insert; it contains no alternative routing or constraint bypass that could explain the mismatch. Therefore the observed acceptance and parent/child divergence occur in the execution engine selected by the module graph. This PR changes go.mod:12 from go-mysql-server e49664d6062c to e787bab6e784, and go.sum records the matching checksum, so the changed dependency selection is a direct practical cause of the behavior under test. The smallest fix is to revert or correct that go-mysql-server version until partition routing validates the child bounds before reporting success and preserves the insert atomically.
- **Why this is likely a bug:** A successful INSERT must correspond to a row in the correct partition, and an out-of-range partition key must be rejected before it changes visible state. The recorded result violates both guarantees: the parent count increased without matching child rows, an invalid key was accepted, and constraint failure was deferred until cleanup. This is a production execution defect rather than a browser artifact because the SQL output directly records the command results and the source shows that Doltgres forwards the INSERT to the pinned engine. The dependency pin changed in this PR, so reverting that pin is a targeted mitigation; the engine should otherwise validate routing and child constraints before returning INSERT success.
**Relevant code:**
`testing/go/regression/tests/insert.sql:195-221`
~~~sql
-- Check tuple routing for partitioned tables
-- fail
insert into range_parted values ('a', 0);
-- ok
insert into range_parted values ('a', 1);
insert into range_parted values ('a', 10);
-- fail
insert into range_parted values ('a', 20);
select tableoid::regclass, * from range_parted;
~~~
`server/ast/insert.go:61-123`
~~~go
tableName, err = nodeTableName(ctx, node)
rows, err = nodeSelect(ctx, node.Rows)
return &vitess.Insert{
Table: tableName,
Columns: columns,
Rows: rows,
}, nil
~~~
`go.mod:12`
~~~go
github.com/dolthub/go-mysql-server v0.20.1-0.20260803232249-e787bab6e784
~~~
Commit: SummaryCoverage spans core database behavior, including schema changes and data updates, value and type preservation, error recovery, startup and restart behavior, persistence, connection readiness, database setup, and clean shutdown. It includes normal workflows plus edge cases around invalid input, delayed readiness, startup races, raw values, and protocol-level consistency. Safe to merge — the exercised application behaviors all passed, with no regressions, new failures, or previously flagged failures attributable to this PR. There are no identified merge blockers from this run. Tests run by Ito
Tip Reply with @itoqa to send us feedback on this test run. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.