fix(load): adjust non-strict LOAD DATA values

[LeftHandCold]

What type of PR is this?

• API-change
• BUG
• Improvement
• Documentation
• Feature
• Test and CI
• Code Refactoring

Which issue(s) this PR fixes:

Fixes #25366

What this PR does / why we need it:

LOAD DATA LOCAL INFILE with non-strict SQL mode should accept the adjusted values from the reported MySQL-compatible CSV case instead of aborting the whole load on invalid field conversions.

Semantic contract for this PR:

• Scope is intentionally narrow: LOAD DATA LOCAL INFILE, CSV format, non-strict SQL mode (StrictSqlMode=false).
• Strict LOCAL loads still reject invalid conversions.
• Non-LOCAL loads still keep the existing rejection behavior, even when session sql_mode is non-strict.
• JSONLINE / PARQUET / other LOAD formats are unchanged.
• \N and mapped null fields remain NULL; empty numeric CSV fields in the scoped path load as zero.
• BOOL keeps existing behavior in this focused patch; for example, empty BOOL remains NULL and invalid BOOL is not adjusted.
• Quoted invalid numeric CSV fields keep the existing rejection behavior; this patch does not adjust them.

Adjusted values implemented in scope:

• invalid numeric fields use the numeric prefix, or 0 when no numeric prefix exists
• invalid or zero DATE fields load as MO's internal zero-date value (types.Date(0)); BVT/JDBC result rendering shows it as 0001-01-01
• over-width CHAR / VARCHAR values truncate by rune width

Intentionally out of scope for this focused compatibility patch:

• warning list / SHOW WARNINGS reporting
• numeric range or unsigned overflow saturation
• invalid BOOL semantics
• broader quoted numeric compatibility beyond preserving existing invalid-value rejection behavior
• empty temporal field behavior
• non-CSV and non-LOCAL LOAD compatibility changes

Changes:

• narrow adjusted-value handling to local CSV non-strict LOAD only
• add unit coverage proving local CSV non-strict adjusts, while local strict, non-LOCAL non-strict, JSONLINE non-strict, and quoted invalid numeric fields still reject, and empty BOOL remains unchanged
• add BVT coverage for the real LOAD DATA LOCAL INFILE SQL path, including the issue sample, \N vs empty numeric fields, strict LOCAL rejection, non-LOCAL non-strict rejection, and quoted invalid numeric rejection

Validation:

CGO flags set to the local MO cgo and thirdparties outputs.

go build ./pkg/sql/colexec/external
go vet ./pkg/sql/colexec/external

go test -v -count=1 -timeout 120s ./pkg/sql/colexec/external -run 'Test(GetOneRowDataLoadDataNonStrictAdjustedValues|GetColDataLoadDataStrictRejectsInvalidInteger|GetColDataLoadDataAdjustmentsOnlyLocalCSVNonStrict|GetColDataLoadDataNonStrictAdjustmentRejectsQuotedInvalidNumeric|GetColDataLoadDataNonStrictAdjustmentLeavesEmptyBoolNull|GetColDataParallelLoadEmptyNumericAsZero|GetColDataParallelLoadNullNumericRemainsNull|GetColDataLoadDataYear)|Test_getColData_StringWidthStrict'
PASS
ok  	github.com/matrixorigin/matrixone/pkg/sql/colexec/external	0.084s

go test -v -count=1 -timeout 120s ./pkg/sql/colexec/external
PASS
ok  	github.com/matrixorigin/matrixone/pkg/sql/colexec/external	0.252s

[qodo-code-review]

Qodo reviews are paused for this user.

Troubleshooting steps vary by plan Learn more →

On a Teams plan?
Reviews resume once this user has a paid seat and their Git account is linked in Qodo.
Link Git account →

Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center?
These require an Enterprise plan - Contact us
Contact us →

[XuPeng-SH]

Requesting changes.

I verified the external package tests locally (with the MO CGo/rpath setup) and CI is green, but I do not think this PR has a closed semantic contract yet.

1. #25366 is still needs-triage, and the issue discussion explicitly says LOAD DATA invalid adjusted-value handling needs a product/R&D decision before implementation. This PR directly changes that contract. Before merging, please document/confirm the intended contract: strict vs non-strict, LOCAL vs non-LOCAL, accepted rows, adjusted values, warning/count reporting, and which LOAD formats are included.

2. The adjustment gate is broader than the reported CSV LOCAL case. shouldApplyLoadDataNonStrictAdjustments only checks ExternType_LOAD && !checkLineStrict (external.go:900-904), then getColData applies numeric/date/string rewrites to any matching LOAD path (external.go:1071-1110). That also changes JSONLINE and other non-strict LOAD surfaces, not just the issue sample. Please either narrow the gate to the intended surface or add explicit coverage/documentation that the broader behavior is intended.

3. The behavior is only a partial MySQL-style adjusted-value implementation. It handles numeric prefix/no-prefix, invalid DATE, and CHAR/VARCHAR truncation for the sample, but warning reporting is missing; numeric range/unsigned overflow still aborts through the existing ErrRange paths; empty temporal fields still go through the NULL path; invalid BOOL and quoted numeric cases can still abort. Please either define these as intentionally out of scope in the contract/tests/PR body, or complete the semantic matrix.

4. The test coverage is not sufficient for a compatibility semantic change. The new tests call getOneRowData/getColData directly, but there is no BVT/e2e coverage for the actual LOAD DATA LOCAL INFILE SQL from the issue. Please add coverage for the real SQL path and unhappy cases: non-strict LOCAL success, strict/non-LOCAL rejection, explicit \\N vs empty fields, JSONLINE unchanged or intentionally adjusted, range/unsigned/bool/quoted numeric/date cases according to the chosen contract.

[XuPeng-SH]

Requesting changes on the current head (ef71d37). The revised scope is much better than the previous version: the adjustment gate is now limited to LOAD DATA LOCAL + CSV + non-strict mode, CI is green, and the new BVT covers the real SQL path. I still found one semantic leak that should be closed before merge.

Confirmed issue: quoted invalid numeric fields are now adjusted even though the PR declares quoted numeric edge cases out of scope.

getColData applies the non-strict numeric-prefix rewrite before the typed parser:

• pkg/sql/colexec/external/external.go:1102-1105 rewrites every scoped numeric value with loadDataNonStrictNumericPrefix.
• The existing integer parser only rejects quoted numeric conversion when parsing fails and then sees field.HasStringQuote (external.go:1209-1211, and the same pattern for other integer widths).
• After this PR, a quoted invalid value like "abc" is rewritten to "0"; strconv.ParseInt("0") succeeds, so the HasStringQuote rejection path is never reached.

I verified this locally with a temporary UT against the current PR head:

err := getColData(
    bat,
    []csvparser.Field{{Val: "abc", HasStringQuote: true}},
    0,
    param, // LOAD DATA LOCAL, CSV, StrictSqlMode=false
    proc.Mp(),
    plan.ExternAttr{ColName: "i", ColIndex: 0, ColFieldIndex: 0},
    proc,
)
require.Error(t, err) // fails: err is nil, value is loaded as 0

This is not just missing coverage; it changes an existing quoted-field contract while the PR body says quoted numeric edge cases are intentionally not part of this compatibility patch.

Suggested fix: if quoted numeric behavior is truly out of scope, skip the numeric-prefix adjustment when field.HasStringQuote is true and add UT/BVT coverage for quoted "abc" / "10x" with FIELDS ... ENCLOSED BY '"' or OPTIONALLY ENCLOSED BY '"', plus strict/non-LOCAL rejection coverage. If the intended behavior is to adjust quoted invalid numeric values too, then please update the semantic contract and add explicit SQL coverage for that expanded behavior.

Validation I ran:

go test -ldflags="-extldflags '-L$(pwd)/cgo -lmo -L$(pwd)/thirdparties/install/lib -Wl,-rpath,@executable_path/lib'" \
  -v -count=1 -timeout 120s ./pkg/sql/colexec/external \
  -run 'Test(GetOneRowDataLoadDataNonStrictAdjustedValues|GetColDataLoadDataStrictRejectsInvalidInteger|GetColDataLoadDataAdjustmentsOnlyLocalCSVNonStrict|GetColDataLoadDataNonStrictAdjustmentLeavesEmptyBoolNull|GetColDataParallelLoadEmptyNumericAsZero|GetColDataParallelLoadNullNumericRemainsNull|GetColDataLoadDataYear)|Test_getColData_StringWidthStrict'
PASS

Temporary quoted-invalid-numeric repro test: FAIL, err is nil.

[aunjgr]

Carefully scoped. The gate shouldApplyLoadDataNonStrictAdjustments correctly restricts to LOCAL + CSV + non-strict. Minor suggestions:

1. T_date gets zero-value adjustment but T_datetime/T_timestamp don't. The PR body says this is out of scope. Add a comment near the temporal handling noting the intentional gap so future maintainers don't think it's an oversight.

2. shouldLoadEmptyNumericAsZero uses isLoadNumericAdjustedValueType (excludes T_bool) vs isLoadNumericZeroFillType (includes T_bool). The PR body says empty BOOL remains NULL — this is correct but deserves a comment at the call site explaining why the two paths diverge.