From 0d6a57eddd5a0913823400531165396a9a4d8e31 Mon Sep 17 00:00:00 2001 From: Brennan Lamey <66885902+brennanjl@users.noreply.github.com> Date: Tue, 18 Feb 2025 14:19:38 -0600 Subject: [PATCH] Engine empty array (#1388) * engine: fix empty array handling * uncomment tests * fix unit tests --- app/node/start.go | 19 +++++++- node/engine/interpreter/interpreter_test.go | 52 ++++++++++++++++----- node/engine/parse/antlr.go | 12 ++--- node/engine/planner/logical/errors.go | 1 + node/engine/planner/logical/planner.go | 8 +++- node/engine/planner/logical/planner_test.go | 12 +++++ node/pg/db_live_test.go | 24 ++++++++++ 7 files changed, 107 insertions(+), 21 deletions(-) diff --git a/app/node/start.go b/app/node/start.go index bf3993bab..60ad6d1f0 100644 --- a/app/node/start.go +++ b/app/node/start.go @@ -42,7 +42,24 @@ func StartCmd() *cobra.Command { // we don't need to worry about order of priority with applying the extension // flag configs because flags are always highest priority - cfg.Extensions = extConfs + + // we merge the flags here because we don't want to totally delete all + // other extension flags. For example, if we have the extension + // "my_ext" configured with key "foo" and value "bar" in the config file, + // and we pass the flag "--extension.erc20.rpc=http://localhost:8545", + // we want to keep the "foo" key in the "my_ext" extension. + for extName, extConf := range extConfs { + existing, ok := cfg.Extensions[extName] + if !ok { + existing = make(map[string]string) + } + + for k, v := range extConf { + existing[k] = v + } + + cfg.Extensions[extName] = existing + } bind.Debugf("effective node config (toml):\n%s", bind.LazyPrinter(func() string { rawToml, err := cfg.ToTOML() diff --git a/node/engine/interpreter/interpreter_test.go b/node/engine/interpreter/interpreter_test.go index 22576ac9d..3bd05807d 100644 --- a/node/engine/interpreter/interpreter_test.go +++ b/node/engine/interpreter/interpreter_test.go @@ -53,19 +53,16 @@ func Test_SQL(t *testing.T) { name string // name of the test // array of sql statements, first element is the namespace, second is the sql statement // they can begin with {namespace}sql, or just sql - sql []string - execSQL string // sql to return the results. Either this or execAction must be set - execVars map[string]any // variables to pass to the execSQL - results [][]any // table of results - err error // expected error, can be nil. Errors _MUST_ occur on the exec. This is a direct match - errContains string // expected error message, can be empty. Errors _MUST_ occur on the exec. This is a substring match - caller string // caller to use for the action, will default to defaultCaller + sql []string + execSQL string // sql to return the results. Either this or execAction must be set + execVars map[string]any // variables to pass to the execSQL + results [][]any // table of results + err error // expected error, can be nil. Errors _MUST_ occur on the exec. This is a direct match + errContains string // expected error message, can be empty. Errors _MUST_ occur on the exec. This is a substring match + caller string // caller to use for the action, will default to defaultCaller + skipInitTables bool // skip the creation of the users and posts tables } - // this is for debugging. - // It helps me skip the users and posts table creation - skipInitTables := false - tests := []testcase{ { name: "insert and select", @@ -714,6 +711,18 @@ func Test_SQL(t *testing.T) { execSQL: `ALTER TABLE tbl DROP column id;`, err: engine.ErrCannotAlterPrimaryKey, }, + { + name: "select empty array (untyped)", + execSQL: `SELECT ARRAY[];`, + err: engine.ErrQueryPlanner, + skipInitTables: true, + }, + { + name: "select empty array (type casted)", + execSQL: `SELECT ARRAY[]::TEXT[];`, + results: [][]any{{[]*string{}}}, + skipInitTables: true, + }, } db := newTestDB(t, nil, nil) @@ -725,7 +734,7 @@ func Test_SQL(t *testing.T) { require.NoError(t, err) defer tx.Rollback(ctx) // always rollback - interp := newTestInterp(t, tx, test.sql, !skipInitTables) + interp := newTestInterp(t, tx, test.sql, !test.skipInitTables) var values [][]any err = interp.Execute(newEngineCtx(test.caller), tx, test.execSQL, test.execVars, func(v *common.Row) error { @@ -1965,6 +1974,25 @@ func Test_Actions(t *testing.T) { caller: "some_user", err: engine.ErrDoesNotHavePrivilege, }, + { + name: "empty array", + stmt: []string{ + `CREATE ACTION empty_array() public view { + $arr := array[]; + }`, + }, + action: "empty_array", + err: engine.ErrArrayDimensionality, + }, + { + name: "empty array (typecasted)", + stmt: []string{ + `CREATE ACTION empty_array() public view { + $arr := array[]::text[]; + }`, + }, + action: "empty_array", + }, } db := newTestDB(t, nil, nil) diff --git a/node/engine/parse/antlr.go b/node/engine/parse/antlr.go index faa3a7e17..8775ff2ea 100644 --- a/node/engine/parse/antlr.go +++ b/node/engine/parse/antlr.go @@ -1786,8 +1786,9 @@ func (s *schemaVisitor) VisitUnary_sql_expr(ctx *gen.Unary_sql_exprContext) any } func (s *schemaVisitor) VisitMake_array_sql_expr(ctx *gen.Make_array_sql_exprContext) any { - e := &ExpressionMakeArray{ - Values: ctx.Sql_expr_list().Accept(s).([]Expression), + e := &ExpressionMakeArray{} + if ctx.Sql_expr_list() != nil { + e.Values = ctx.Sql_expr_list().Accept(s).([]Expression) } if ctx.Type_cast() != nil { @@ -2116,12 +2117,9 @@ func (s *schemaVisitor) VisitMake_array_action_expr(ctx *gen.Make_array_action_e // directly below. } - // we could enforce this in the parser, but it is not super intuitive, - // so we want to control the error message - if ctx.Action_expr_list() == nil { - s.errs.RuleErr(ctx, ErrSyntax, "cannot assign empty arrays. declare using `$arr type[];` instead`") + if ctx.Action_expr_list() != nil { + e.Values = ctx.Action_expr_list().Accept(s).([]Expression) } - e.Values = ctx.Action_expr_list().Accept(s).([]Expression) if ctx.Type_cast() != nil { e.TypeCast = ctx.Type_cast().Accept(s).(*types.DataType) diff --git a/node/engine/planner/logical/errors.go b/node/engine/planner/logical/errors.go index d9b8ef5f9..5aa9e63af 100644 --- a/node/engine/planner/logical/errors.go +++ b/node/engine/planner/logical/errors.go @@ -19,4 +19,5 @@ var ( ErrWindowNotDefined = errors.New("window not defined") ErrFunctionDoesNotExist = errors.New("function does not exist") ErrActionInSQLStmt = errors.New("actions cannot be used in SQL statements") + ErrUntypedEmptyArray = errors.New("cannot detect type for empty array") ) diff --git a/node/engine/planner/logical/planner.go b/node/engine/planner/logical/planner.go index 9a3343dd0..5867c4d28 100644 --- a/node/engine/planner/logical/planner.go +++ b/node/engine/planner/logical/planner.go @@ -1334,7 +1334,13 @@ func (s *scopeContext) exprWithAggRewrite(node parse.Expression, currentRel *Rel }, field2) case *parse.ExpressionMakeArray: if len(node.Values) == 0 { - return nil, nil, false, fmt.Errorf("array constructor must have at least one element") + // if there are no values, we cannot determine the type unless + // it is type casted + if node.TypeCast == nil { + return nil, nil, false, fmt.Errorf("%w: Try type casting to the desired type, for example ARRAY[]::TEXT[]", ErrUntypedEmptyArray) + } + + return cast(&ArrayConstructor{}, anonField(node.TypeCast)) } var exprs []Expression diff --git a/node/engine/planner/logical/planner_test.go b/node/engine/planner/logical/planner_test.go index a4adb064e..4bfa81b50 100644 --- a/node/engine/planner/logical/planner_test.go +++ b/node/engine/planner/logical/planner_test.go @@ -660,6 +660,18 @@ func Test_Planner(t *testing.T) { }, err: logical.ErrActionInSQLStmt, }, + { + name: "select empty array", + sql: "select array[]", + err: logical.ErrUntypedEmptyArray, + }, + { + name: "select empty array with type", + sql: "select array[]::int[]", + wt: "Return: ?column? [int8[]]\n" + + "└─Project: []::int8\n" + + " └─Empty Scan\n", + }, } for _, test := range tests { diff --git a/node/pg/db_live_test.go b/node/pg/db_live_test.go index 1c0eb7bf5..028634c7b 100644 --- a/node/pg/db_live_test.go +++ b/node/pg/db_live_test.go @@ -1847,3 +1847,27 @@ func Test_CancelListen(t *testing.T) { require.Len(t, received, 10) } + +func Test_DeleteMe(t *testing.T) { + ctx := context.Background() + + db, err := NewDB(ctx, cfg) + require.NoError(t, err) + defer db.Close() + + tx, err := db.BeginTx(ctx) + require.NoError(t, err) + defer tx.Rollback(ctx) // always rollback + + _, err = tx.Execute(ctx, "CREATE TABLE users (id serial primary key, name text)", QueryModeExec) + require.NoError(t, err) + + ids := []any{} + res := []any{ + &ids, + } + err = QueryRowFunc(ctx, tx, "SELECT array_agg(id) from users", res, func() error { + return nil + }) + require.NoError(t, err) +}