clean up impl

Author: elianddb

enginetest/memory_engine_test.go

@@ -200,68 +200,23 @@ func TestSingleQueryPrepared(t *testing.T) {
 
 // Convenience test for debugging a single query. Unskip and set to the desired query.
 func TestSingleScript(t *testing.T) {
-	// t.Skip()
+	t.Skip()
 	var scripts = []queries.ScriptTest{
 		{
-			// https://github.com/dolthub/dolt/issues/9873
-			Name: "FOR UPDATE OF syntax support tests",
-			SetUpScript: []string{
-				"CREATE TABLE task_instance (id INT PRIMARY KEY, task_id VARCHAR(255), dag_id VARCHAR(255), run_id VARCHAR(255), state VARCHAR(50), queued_by_job_id INT)",
-				"CREATE TABLE job (id INT PRIMARY KEY, state VARCHAR(50))",
-				"CREATE TABLE dag_run (dag_id VARCHAR(255), run_id VARCHAR(255), state VARCHAR(50))",
-				"CREATE TABLE t (id INT PRIMARY KEY, name VARCHAR(50))",
-				"INSERT INTO task_instance VALUES (1, 'task1', 'dag1', 'run1', 'running', 1)",
-				"INSERT INTO job VALUES (1, 'running')",
-				"INSERT INTO dag_run VALUES ('dag1', 'run1', 'running')",
-				"INSERT INTO t VALUES (1, 'test')",
-			},
+			Name:        "AS OF propagates to nested CALLs",
+			SetUpScript: []string{},
 			Assertions: []queries.ScriptTestAssertion{
 				{
-					Query: `SELECT task_instance.id, task_instance.task_id, task_instance.dag_id, task_instance.run_id
-FROM task_instance INNER JOIN job ON job.id = task_instance.queued_by_job_id INNER JOIN dag_run ON dag_run.dag_id = task_instance.dag_id AND dag_run.run_id = task_instance.run_id
- WHERE task_instance.state IN ('running', 'queued', 'scheduled') AND NOT (job.state <=> 'running') AND dag_run.state = 'running' FOR UPDATE OF task_instance SKIP LOCKED`,
-					Expected: []sql.Row{},
-				},
-				{
-					Query:    "SELECT * FROM t FOR UPDATE",
-					Expected: []sql.Row{{1, "test"}},
-				},
-				{
-					Query:    "SELECT * FROM t FOR UPDATE OF t",
-					Expected: []sql.Row{{1, "test"}},
-				},
-				{
-					Query:    "SELECT * FROM t FOR UPDATE OF t SKIP LOCKED",
-					Expected: []sql.Row{{1, "test"}},
-				},
-				{
-					Query:    "SELECT * FROM t FOR UPDATE OF t NOWAIT",
-					Expected: []sql.Row{{1, "test"}},
-				},
-				{
-					Query:    "SELECT * FROM task_instance t1, job t2 FOR UPDATE OF t1, t2",
-					Expected: []sql.Row{{1, "task1", "dag1", "run1", "running", 1, 1, "running"}},
-				},
-				{
-					Query:       "SELECT * FROM t FOR UPDATE OF nonexistent_table",
-					ExpectedErr: sql.ErrUnresolvedTableLock,
-				},
-				{
-					Query:          "SELECT * FROM t FOR UPDATE OF t, nonexistent_table",
-					ExpectedErr:    sql.ErrUnresolvedTableLock,
-					ExpectedErrStr: fmt.Sprintf(sql.ErrUnresolvedTableLock.Message, "nonexistent_table"),
-				},
-				{
-					Query:       "SELECT * FROM t FOR UPDATE OF",
-					ExpectedErr: sql.ErrSyntaxError,
-				},
-				{
-					Query:       "SELECT * FROM t FOR UPDATE test",
-					ExpectedErr: sql.ErrSyntaxError,
+					Query: "create procedure create_proc() create table t (i int primary key, j int);",
+					Expected: []sql.Row{
+						{types.NewOkResult(0)},
+					},
 				},
 				{
-					Query:    "SELECT * FROM t FOR UPDATE",
-					Expected: []sql.Row{{1, "test"}},
+					Query: "call create_proc()",
+					Expected: []sql.Row{
+						{types.NewOkResult(0)},
+					},
 				},
 			},
 		},

enginetest/queries/script_queries.go

@@ -124,6 +124,7 @@ type ScriptTestAssertion struct {
 var ScriptTests = []ScriptTest{
 	{
 		// https://github.com/dolthub/dolt/issues/9873
+		// TODO: `FOR UPDATE OF` (`FOR UPDATE` in general) is currently a no-op: https://www.dolthub.com/blog/2023-10-23-hold-my-beer/
 		Name: "FOR UPDATE OF syntax support tests",
 		SetUpScript: []string{
 			"CREATE TABLE task_instance (id INT PRIMARY KEY, task_id VARCHAR(255), dag_id VARCHAR(255), run_id VARCHAR(255), state VARCHAR(50), queued_by_job_id INT)",

sql/planbuilder/select.go

@@ -123,9 +123,7 @@ func (b *Builder) buildSelect(inScope *scope, s *ast.Select) (outScope *scope) {
 		outScope.node = l
 	}
 
-	if s.Lock != nil {
-		b.buildForUpdateOf(s.Lock, fromScope)
-	}
+	b.buildForUpdateOf(s.Lock, fromScope)
 
 	return
 }
@@ -243,7 +241,10 @@ func (b *Builder) renameSource(scope *scope, table string, cols []string) {
 	}
 }
 
-// buildForUpdateOf builds the FOR UPDATE OF clause. Currently a no-op that validates table names.
+// buildForUpdateOf builds the `FOR UPDATE OF` clause, ensuring that all tables listed are
+// present in the clause. `FOR UPDATE` in general is a no-op, so `FOR UPDATE OF` is
+// also a no-op: https://www.dolthub.com/blog/2023-10-23-hold-my-beer/
+// TODO: implement actual row-level locking for `FOR UPDATE` clauses in general.
 func (b *Builder) buildForUpdateOf(lock *ast.Lock, fromScope *scope) {
 	if lock == nil {
 		return
@@ -257,6 +258,4 @@ func (b *Builder) buildForUpdateOf(lock *ast.Lock, fromScope *scope) {
 			return
 		}
 	}
-
-	// This is currently a no-op: https://www.dolthub.com/blog/2023-10-23-hold-my-beer/
 }