From 61b66f7be663b68398e4b6e303dfaa475f69697d Mon Sep 17 00:00:00 2001 From: Jason Fulghum Date: Mon, 22 Jan 2024 12:30:50 -0800 Subject: [PATCH 1/2] Moving validation for InsertInto nodes into a new analyzer rule that runs during the validation batch. --- enginetest/queries/load_queries.go | 7 ++- sql/analyzer/inserts.go | 26 ----------- sql/analyzer/rule_ids.go | 1 + sql/analyzer/ruleid_string.go | 17 ++++---- sql/analyzer/rules.go | 1 + sql/analyzer/validation_rules.go | 69 ++++++++++++++++++++++++++++++ 6 files changed, 83 insertions(+), 38 deletions(-) diff --git a/enginetest/queries/load_queries.go b/enginetest/queries/load_queries.go index 1f29fadf20..58d62ba71c 100644 --- a/enginetest/queries/load_queries.go +++ b/enginetest/queries/load_queries.go @@ -43,8 +43,7 @@ var LoadDataScripts = []ScriptTest{ }, Assertions: []ScriptTestAssertion{ { - Query: "LOAD DATA INFILE './testdata/test1.txt' INTO TABLE loadtable FIELDS ENCLOSED BY '\"'", - + Query: "LOAD DATA INFILE './testdata/test1.txt' INTO TABLE loadtable FIELDS ENCLOSED BY '\"'", ExpectedErrStr: "Check constraint \"loadtable_chk_1\" violated", }, }, @@ -275,11 +274,11 @@ var LoadDataErrorScripts = []ScriptTest{ { Name: "Load data with unknown columns throws an error", SetUpScript: []string{ - "create table loadtable(pk int primary key)", + "create table loadtable(pk int primary key, i int)", }, Assertions: []ScriptTestAssertion{ { - Query: "LOAD DATA INFILE './testdata/test1.txt' INTO TABLE loadtable FIELDS ENCLOSED BY '\"' (bad)", + Query: "LOAD DATA INFILE './testdata/test1.txt' INTO TABLE loadtable FIELDS ENCLOSED BY '\"' (pk, fake_col, i)", ExpectedErr: plan.ErrInsertIntoNonexistentColumn, }, }, diff --git a/sql/analyzer/inserts.go b/sql/analyzer/inserts.go index a94c8557cc..9c66a30d06 100644 --- a/sql/analyzer/inserts.go +++ b/sql/analyzer/inserts.go @@ -45,22 +45,6 @@ func resolveInsertRows(ctx *sql.Context, a *Analyzer, n sql.Node, scope *plan.Sc return nil, transform.SameTree, err } - if insert.IsReplace { - var ok bool - _, ok = insertable.(sql.ReplaceableTable) - if !ok { - return nil, transform.SameTree, plan.ErrReplaceIntoNotSupported.New() - } - } - - if len(insert.OnDupExprs) > 0 { - var ok bool - _, ok = insertable.(sql.UpdatableTable) - if !ok { - return nil, transform.SameTree, plan.ErrOnDuplicateKeyUpdateNotSupported.New() - } - } - source := insert.Source // TriggerExecutor has already been analyzed if _, ok := insert.Source.(*plan.TriggerExecutor); !ok { @@ -93,16 +77,6 @@ func resolveInsertRows(ctx *sql.Context, a *Analyzer, n sql.Node, scope *plan.Sc for i, f := range dstSchema { columnNames[i] = f.Name } - } else { - err = validateColumns(table.Name(), columnNames, dstSchema, source) - if err != nil { - return nil, transform.SameTree, err - } - } - - err = validateValueCount(columnNames, source) - if err != nil { - return nil, transform.SameTree, err } // The schema of the destination node and the underlying table differ subtly in terms of defaults diff --git a/sql/analyzer/rule_ids.go b/sql/analyzer/rule_ids.go index 8ad6579ffb..9140de2b96 100644 --- a/sql/analyzer/rule_ids.go +++ b/sql/analyzer/rule_ids.go @@ -137,6 +137,7 @@ const ( validateUnionSchemasMatchId // validateUnionSchemasMatch validateAggregationsId // validateAggregations validateDeleteFromId // validateDeleteFrom + validateInsertsId // validateInserts // after all cacheSubqueryResultsId // cacheSubqueryResults diff --git a/sql/analyzer/ruleid_string.go b/sql/analyzer/ruleid_string.go index 4e7812b445..6d09ca4546 100755 --- a/sql/analyzer/ruleid_string.go +++ b/sql/analyzer/ruleid_string.go @@ -133,17 +133,18 @@ func _() { _ = x[validateUnionSchemasMatchId-122] _ = x[validateAggregationsId-123] _ = x[validateDeleteFromId-124] - _ = x[cacheSubqueryResultsId-125] - _ = x[cacheSubqueryAliasesInJoinsId-126] - _ = x[AutocommitId-127] - _ = x[TrackProcessId-128] - _ = x[parallelizeId-129] - _ = x[clearWarningsId-130] + _ = x[validateInsertsId-125] + _ = x[cacheSubqueryResultsId-126] + _ = x[cacheSubqueryAliasesInJoinsId-127] + _ = x[AutocommitId-128] + _ = x[TrackProcessId-129] + _ = x[parallelizeId-130] + _ = x[clearWarningsId-131] } -const _RuleId_name = "applyDefaultSelectLimitvalidateOffsetAndLimitvalidateStarExpressionsvalidateCreateTablevalidateAlterTablevalidateExprSemresolveVariablesresolveNamedWindowsresolveSetVariablesresolveViewsliftCtesresolveCtesliftRecursiveCtesresolveDatabasesresolveTablesloadStoredProceduresvalidateDropTablespruneDropTablessetTargetSchemasresolveCreateLikeparseColumnDefaultsresolveDropConstraintvalidateDropConstraintloadCheckConstraintsassignCatalogresolveAnalyzeTablesresolveCreateSelectresolveSubqueriessetViewTargetSchemaresolveUnionsresolveDescribeQuerycheckUniqueTableNamesresolveTableFunctionsresolveDeclarationsresolveColumnDefaultsvalidateColumnDefaultsvalidateCreateTriggervalidateCreateProcedureresolveCreateProcedureloadInfoSchemavalidateReadOnlyDatabasevalidateReadOnlyTransactionvalidateDatabaseSetvalidatePrivilegesreresolveTablessetInsertColumnsvalidateJoinComplexityapplyBinlogReplicaControllerapplyEventSchedulerresolveUsingJoinsresolveOrderbyLiteralsresolveFunctionsflattenTableAliasespushdownSortpushdownGroupbyAliasespushdownSubqueryAliasFiltersqualifyColumnsresolveColumnsvalidateCheckConstraintresolveBarewordSetVariablesreplaceCountStarexpandStarstransposeRightJoinsresolveHavingmergeUnionSchemasflattenAggregationExprsreorderProjectionresolveSubqueryExprsreplaceCrossJoinsmoveJoinCondsToFiltermoveFiltersToJoinCondsimplifyFilterspushNotFiltersoptimizeDistincthoistOutOfScopeFiltersunnestInSubqueriesunnestExistsSubqueriesfinalizeSubqueriesfinalizeUnionsloadTriggersloadEventsprocessTruncateresolveAlterColumnresolveGeneratorsremoveUnnecessaryConvertspruneColumnsstripTableNamesFromColumnDefaultsfoldEmptyJoinsoptimizeJoinsgenerateIndexScansmatchAgainstpushFiltersapplyIndexesFromOuterScopepruneTablesfixupAuxiliaryExprsassignExecIndexesinlineSubqueryAliasRefseraseProjectionflattenDistinctreplaceAggreplaceIdxSortinsertTopNapplyHashInresolveInsertRowsresolvePreparedInsertapplyTriggersapplyProceduresassignRoutinesmodifyUpdateExprsForJoinapplyRowUpdateAccumulatorsrollback triggersapplyFKsvalidateResolvedvalidateOrderByvalidateGroupByvalidateSchemaSourcevalidateIndexCreationvalidateOperandsvalidateCaseResultTypesvalidateIntervalUsagevalidateExplodeUsagevalidateSubqueryColumnsvalidateUnionSchemasMatchvalidateAggregationsvalidateDeleteFromcacheSubqueryResultscacheSubqueryAliasesInJoinsaddAutocommitNodetrackProcessparallelizeclearWarnings" +const _RuleId_name = "applyDefaultSelectLimitvalidateOffsetAndLimitvalidateStarExpressionsvalidateCreateTablevalidateAlterTablevalidateExprSemresolveVariablesresolveNamedWindowsresolveSetVariablesresolveViewsliftCtesresolveCtesliftRecursiveCtesresolveDatabasesresolveTablesloadStoredProceduresvalidateDropTablespruneDropTablessetTargetSchemasresolveCreateLikeparseColumnDefaultsresolveDropConstraintvalidateDropConstraintloadCheckConstraintsassignCatalogresolveAnalyzeTablesresolveCreateSelectresolveSubqueriessetViewTargetSchemaresolveUnionsresolveDescribeQuerycheckUniqueTableNamesresolveTableFunctionsresolveDeclarationsresolveColumnDefaultsvalidateColumnDefaultsvalidateCreateTriggervalidateCreateProcedureresolveCreateProcedureloadInfoSchemavalidateReadOnlyDatabasevalidateReadOnlyTransactionvalidateDatabaseSetvalidatePrivilegesreresolveTablessetInsertColumnsvalidateJoinComplexityapplyBinlogReplicaControllerapplyEventSchedulerresolveUsingJoinsresolveOrderbyLiteralsresolveFunctionsflattenTableAliasespushdownSortpushdownGroupbyAliasespushdownSubqueryAliasFiltersqualifyColumnsresolveColumnsvalidateCheckConstraintresolveBarewordSetVariablesreplaceCountStarexpandStarstransposeRightJoinsresolveHavingmergeUnionSchemasflattenAggregationExprsreorderProjectionresolveSubqueryExprsreplaceCrossJoinsmoveJoinCondsToFiltermoveFiltersToJoinCondsimplifyFilterspushNotFiltersoptimizeDistincthoistOutOfScopeFiltersunnestInSubqueriesunnestExistsSubqueriesfinalizeSubqueriesfinalizeUnionsloadTriggersloadEventsprocessTruncateresolveAlterColumnresolveGeneratorsremoveUnnecessaryConvertspruneColumnsstripTableNamesFromColumnDefaultsfoldEmptyJoinsoptimizeJoinsgenerateIndexScansmatchAgainstpushFiltersapplyIndexesFromOuterScopepruneTablesfixupAuxiliaryExprsassignExecIndexesinlineSubqueryAliasRefseraseProjectionflattenDistinctreplaceAggreplaceIdxSortinsertTopNapplyHashInresolveInsertRowsresolvePreparedInsertapplyTriggersapplyProceduresassignRoutinesmodifyUpdateExprsForJoinapplyRowUpdateAccumulatorsrollback triggersapplyFKsvalidateResolvedvalidateOrderByvalidateGroupByvalidateSchemaSourcevalidateIndexCreationvalidateOperandsvalidateCaseResultTypesvalidateIntervalUsagevalidateExplodeUsagevalidateSubqueryColumnsvalidateUnionSchemasMatchvalidateAggregationsvalidateDeleteFromvalidateInsertscacheSubqueryResultscacheSubqueryAliasesInJoinsaddAutocommitNodetrackProcessparallelizeclearWarnings" -var _RuleId_index = [...]uint16{0, 23, 45, 68, 87, 105, 120, 136, 155, 174, 186, 194, 205, 222, 238, 251, 271, 289, 304, 320, 337, 356, 377, 399, 419, 432, 452, 471, 488, 507, 520, 540, 561, 582, 601, 622, 644, 665, 688, 710, 724, 748, 775, 794, 812, 827, 843, 865, 893, 912, 929, 951, 967, 986, 998, 1020, 1048, 1062, 1076, 1099, 1126, 1142, 1153, 1172, 1185, 1202, 1225, 1242, 1262, 1279, 1300, 1321, 1336, 1350, 1366, 1388, 1406, 1428, 1446, 1460, 1472, 1482, 1497, 1515, 1532, 1557, 1569, 1602, 1616, 1629, 1647, 1659, 1670, 1696, 1707, 1726, 1743, 1766, 1781, 1796, 1806, 1820, 1830, 1841, 1858, 1879, 1892, 1907, 1921, 1945, 1971, 1988, 1996, 2012, 2027, 2042, 2062, 2083, 2099, 2122, 2143, 2163, 2186, 2211, 2231, 2249, 2269, 2296, 2313, 2325, 2336, 2349} +var _RuleId_index = [...]uint16{0, 23, 45, 68, 87, 105, 120, 136, 155, 174, 186, 194, 205, 222, 238, 251, 271, 289, 304, 320, 337, 356, 377, 399, 419, 432, 452, 471, 488, 507, 520, 540, 561, 582, 601, 622, 644, 665, 688, 710, 724, 748, 775, 794, 812, 827, 843, 865, 893, 912, 929, 951, 967, 986, 998, 1020, 1048, 1062, 1076, 1099, 1126, 1142, 1153, 1172, 1185, 1202, 1225, 1242, 1262, 1279, 1300, 1321, 1336, 1350, 1366, 1388, 1406, 1428, 1446, 1460, 1472, 1482, 1497, 1515, 1532, 1557, 1569, 1602, 1616, 1629, 1647, 1659, 1670, 1696, 1707, 1726, 1743, 1766, 1781, 1796, 1806, 1820, 1830, 1841, 1858, 1879, 1892, 1907, 1921, 1945, 1971, 1988, 1996, 2012, 2027, 2042, 2062, 2083, 2099, 2122, 2143, 2163, 2186, 2211, 2231, 2249, 2264, 2284, 2311, 2328, 2340, 2351, 2364} func (i RuleId) String() string { if i < 0 || i >= RuleId(len(_RuleId_index)-1) { diff --git a/sql/analyzer/rules.go b/sql/analyzer/rules.go index 8869818a4b..939f70865e 100644 --- a/sql/analyzer/rules.go +++ b/sql/analyzer/rules.go @@ -89,6 +89,7 @@ var DefaultValidationRules = []Rule{ {validateSubqueryColumnsId, validateSubqueryColumns}, {validateUnionSchemasMatchId, validateUnionSchemasMatch}, {validateAggregationsId, validateAggregations}, + {validateInsertsId, validateInserts}, } var OnceAfterAll = []Rule{ diff --git a/sql/analyzer/validation_rules.go b/sql/analyzer/validation_rules.go index f116877e47..9d96c2f907 100644 --- a/sql/analyzer/validation_rules.go +++ b/sql/analyzer/validation_rules.go @@ -834,6 +834,75 @@ func validateAggregations(ctx *sql.Context, a *Analyzer, n sql.Node, scope *plan return n, transform.SameTree, validationErr } +// validateInserts validates InsertInto nodes. +func validateInserts(_ *sql.Context, _ *Analyzer, n sql.Node, _ *plan.Scope, _ RuleSelector) (sql.Node, transform.TreeIdentity, error) { + if _, ok := n.(*plan.TriggerExecutor); ok { + return n, transform.SameTree, nil + } else if _, ok := n.(*plan.CreateProcedure); ok { + return n, transform.SameTree, nil + } + + return transform.Node(n, func(n sql.Node) (sql.Node, transform.TreeIdentity, error) { + // Skip any nodes that aren't an InsertInto + ii, ok := n.(*plan.InsertInto) + if !ok { + return n, transform.SameTree, nil + } + + table := getResolvedTable(ii.Destination) + + insertable, err := plan.GetInsertable(table) + if err != nil { + return n, transform.SameTree, err + } + + if ii.IsReplace { + var ok bool + _, ok = insertable.(sql.ReplaceableTable) + if !ok { + return n, transform.SameTree, plan.ErrReplaceIntoNotSupported.New() + } + } + + if len(ii.OnDupExprs) > 0 { + var ok bool + _, ok = insertable.(sql.UpdatableTable) + if !ok { + return n, transform.SameTree, plan.ErrOnDuplicateKeyUpdateNotSupported.New() + } + } + + // normalize the column name + dstSchema := insertable.Schema() + columnNames := make([]string, len(ii.ColumnNames)) + for i, name := range ii.ColumnNames { + columnNames[i] = strings.ToLower(name) + } + + // If no columns are given and value tuples are not all empty, use the full schema + if len(columnNames) == 0 && existsNonZeroValueCount(ii.Source) { + columnNames = make([]string, len(dstSchema)) + for i, f := range dstSchema { + columnNames[i] = f.Name + } + } + + if len(ii.ColumnNames) > 0 { + err := validateColumns(table.Name(), columnNames, dstSchema, ii.Source) + if err != nil { + return n, transform.SameTree, err + } + } + + err = validateValueCount(columnNames, ii.Source) + if err != nil { + return nil, transform.SameTree, err + } + + return n, transform.SameTree, nil + }) +} + // checkForAggregationFunctions returns an ErrAggregationUnsupported error if any aggregation // functions are found in the specified expressions. func checkForAggregationFunctions(exprs []sql.Expression) error { From e9321455d4e424f157e3c588eef61f7d72ffe55a Mon Sep 17 00:00:00 2001 From: Jason Fulghum Date: Mon, 22 Jan 2024 13:08:44 -0800 Subject: [PATCH 2/2] Expanding LOAD DATA error test cases for invalid column names --- enginetest/queries/load_queries.go | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/enginetest/queries/load_queries.go b/enginetest/queries/load_queries.go index 58d62ba71c..de29acace3 100644 --- a/enginetest/queries/load_queries.go +++ b/enginetest/queries/load_queries.go @@ -277,10 +277,18 @@ var LoadDataErrorScripts = []ScriptTest{ "create table loadtable(pk int primary key, i int)", }, Assertions: []ScriptTestAssertion{ + { + Query: "LOAD DATA INFILE './testdata/test1.txt' INTO TABLE loadtable FIELDS ENCLOSED BY '\"' (fake_col, pk, i)", + ExpectedErr: plan.ErrInsertIntoNonexistentColumn, + }, { Query: "LOAD DATA INFILE './testdata/test1.txt' INTO TABLE loadtable FIELDS ENCLOSED BY '\"' (pk, fake_col, i)", ExpectedErr: plan.ErrInsertIntoNonexistentColumn, }, + { + Query: "LOAD DATA INFILE './testdata/test1.txt' INTO TABLE loadtable FIELDS ENCLOSED BY '\"' (pk, i, fake_col)", + ExpectedErr: plan.ErrInsertIntoNonexistentColumn, + }, }, }, {