diff --git a/.github/workflows/experimental-github-actions-testing.yml b/.github/workflows/experimental-github-actions-testing.yml index 324f1fa3addd..773d292a9a68 100644 --- a/.github/workflows/experimental-github-actions-testing.yml +++ b/.github/workflows/experimental-github-actions-testing.yml @@ -13,7 +13,7 @@ jobs: # Instead, we want to check out the PR as is. ref: ${{ github.event.pull_request.head.sha }} - run: ./build/github/get-engflow-keys.sh - - run: bazel test //pkg:all_tests --config engflowpublic --config crosslinux --jobs 300 --tls_client_certificate=/home/agent/engflow.crt --tls_client_key=/home/agent/engflow.key --remote_download_minimal + - run: bazel test //pkg:all_tests --config engflowpublic --config crosslinux --jobs 300 --tls_client_certificate=/home/agent/engflow.crt --tls_client_key=/home/agent/engflow.key --remote_download_minimal --bes_keywords=github_pr_number=${{ github.event.pull_request.number }} - name: clean up if: always() run: rm -f /home/agent/engflow.* diff --git a/pkg/ccl/backupccl/testdata/backup-restore/plpgsql_procedures b/pkg/ccl/backupccl/testdata/backup-restore/plpgsql_procedures new file mode 100644 index 000000000000..bb8f95f30875 --- /dev/null +++ b/pkg/ccl/backupccl/testdata/backup-restore/plpgsql_procedures @@ -0,0 +1,433 @@ +# Test backing up and restoring a database with PL/pgSQL procedures. +new-cluster name=s +---- + +exec-sql +CREATE DATABASE db1; +USE db1; +CREATE SCHEMA sc1; +CREATE TABLE sc1.tbl1(a INT PRIMARY KEY); +CREATE TYPE sc1.enum1 AS ENUM('Good'); +CREATE SEQUENCE sc1.sq1; +CREATE PROCEDURE sc1.p1(a sc1.enum1) LANGUAGE PLpgSQL AS $$ + BEGIN + SELECT a FROM sc1.tbl1; + SELECT nextval('sc1.sq1'); + END +$$; +CREATE SCHEMA sc2; +CREATE TABLE sc2.tbl2(a INT PRIMARY KEY); +CREATE PROCEDURE sc2.p2(i INT) LANGUAGE PLpgSQL AS $$ + BEGIN + INSERT INTO sc2.tbl2 VALUES (i); + END +$$; +---- + +exec-sql +CALL sc1.p1('Good'::sc1.enum1) +---- + +exec-sql +CALL sc2.p2(123) +---- + +query-sql +SELECT * FROM sc2.tbl2 +---- +123 + +exec-sql +BACKUP DATABASE db1 INTO 'nodelocal://1/test/' +---- + +query-sql +WITH descs AS ( + SHOW BACKUP LATEST IN 'nodelocal://1/test/' +) +SELECT database_name, parent_schema_name, object_name, object_type, is_full_cluster FROM descs +---- + db1 database false +db1 public schema false +db1 sc1 schema false +db1 sc1 tbl1 table false +db1 sc1 enum1 type false +db1 sc1 _enum1 type false +db1 sc1 sq1 table false +db1 sc1 p1 function false +db1 sc2 schema false +db1 sc2 tbl2 table false +db1 sc2 p2 function false + +query-sql +SELECT create_statement FROM [SHOW CREATE PROCEDURE sc1.p1] +---- +---- +CREATE PROCEDURE sc1.p1(IN a db1.sc1.enum1) + LANGUAGE plpgsql + AS $$ + BEGIN + SELECT a FROM db1.sc1.tbl1; + SELECT nextval('sc1.sq1'::REGCLASS); + END + +$$ +---- +---- + +exec-sql +CALL sc1.p1('Good'::sc1.enum1) +---- + +query-sql +SELECT currval('sc1.sq1') +---- +2 + +exec-sql +DROP DATABASE db1 +---- + +exec-sql +RESTORE DATABASE db1 FROM LATEST IN 'nodelocal://1/test/' WITH new_db_name = db1_new +---- + +exec-sql +USE db1_new +---- + +# Make sure ids in signature and body are rewritten. +# 1. argument type id is rewritten so that type name is deserialized correctly. +# 2. db name in qualified name is rewritten. +# 3. sequence id is rewritten so that sequence name is deserialized correctly. +query-sql +SELECT create_statement FROM [SHOW CREATE PROCEDURE sc1.p1] +---- +---- +CREATE PROCEDURE sc1.p1(IN a db1_new.sc1.enum1) + LANGUAGE plpgsql + AS $$ + BEGIN + SELECT a FROM db1_new.sc1.tbl1; + SELECT nextval('sc1.sq1'::REGCLASS); + END + +$$ +---- +---- + +# Make sure procedure signature is rewritten in schema descriptor so that +# procedure can be resolved and executed. +exec-sql +CALL sc1.p1('Good'::sc1.enum1) +---- + +query-sql +SELECT currval('sc1.sq1') +---- +2 + +# Make sure procedure still inserts into the correct table. +exec-sql +CALL sc2.p2(456) +---- + +query-sql +SELECT * FROM sc2.tbl2 +---- +123 +456 + +# Make sure dependency IDs are rewritten. +# Note that technically this only tests forward-reference IDs in depended-on +# objects are rewritten. But since we have cross-references validation, so this +# also means back-references in the function descriptor are good. +exec-sql +DROP SEQUENCE sc1.sq1 +---- +pq: cannot drop sequence sq1 because other objects depend on it + +exec-sql +DROP TABLE sc1.tbl1 +---- +pq: cannot drop table tbl1 because other objects depend on it + +# TODO(mgartner): The error message should say "procedure". +exec-sql +ALTER TABLE sc1.tbl1 RENAME TO tbl1_new +---- +pq: cannot rename relation "sc1.tbl1" because function "p1" depends on it +HINT: consider dropping "p1" first. + +# TODO(mgartner): The error message should say "procedure". +exec-sql +ALTER TABLE sc1.tbl1 SET SCHEMA sc2; +---- +pq: cannot set schema on relation "tbl1" because function "p1" depends on it +HINT: consider dropping "p1" first. + +exec-sql +DROP TYPE sc1.enum1 +---- +pq: cannot drop type "enum1" because other objects ([db1_new.sc1.p1]) still depend on it + +# Test backing up and restoring a full cluster with procedures. +new-cluster name=s1 +---- + +exec-sql cluster=s1 +CREATE DATABASE db1; +USE db1; +CREATE SCHEMA sc1; +CREATE TABLE sc1.tbl1(a INT PRIMARY KEY); +CREATE TYPE sc1.enum1 AS ENUM('Good'); +CREATE SEQUENCE sc1.sq1; +CREATE PROCEDURE sc1.p1(a sc1.enum1) LANGUAGE PLpgSQL AS $$ + BEGIN + SELECT a FROM sc1.tbl1; + SELECT nextval('sc1.sq1'); + END +$$; +CREATE SCHEMA sc2; +CREATE TABLE sc2.tbl2(a INT PRIMARY KEY); +CREATE PROCEDURE sc2.p2(i INT) LANGUAGE PLpgSQL AS $$ + BEGIN + INSERT INTO sc2.tbl2 VALUES (i); + END +$$; +---- + +exec-sql +CALL sc1.p1('Good'::sc1.enum1) +---- + +exec-sql +CALL sc2.p2(123) +---- + +query-sql +SELECT * FROM sc2.tbl2 +---- +123 + +exec-sql +BACKUP INTO 'nodelocal://1/test/' +---- + +query-sql +WITH descs AS ( + SHOW BACKUP LATEST IN 'nodelocal://1/test/' +) +SELECT + database_name, parent_schema_name, object_name, object_type, is_full_cluster +FROM + descs +WHERE + database_name = 'db1' + +---- +db1 public schema true +db1 sc1 schema true +db1 sc1 tbl1 table true +db1 sc1 enum1 type true +db1 sc1 _enum1 type true +db1 sc1 sq1 table true +db1 sc1 p1 function true +db1 sc2 schema true +db1 sc2 tbl2 table true +db1 sc2 p2 function true + +query-sql +SELECT create_statement FROM [SHOW CREATE PROCEDURE sc1.p1] +---- +---- +CREATE PROCEDURE sc1.p1(IN a db1.sc1.enum1) + LANGUAGE plpgsql + AS $$ + BEGIN + SELECT a FROM db1.sc1.tbl1; + SELECT nextval('sc1.sq1'::REGCLASS); + END + +$$ +---- +---- + +query-sql +CALL sc1.p1('Good'::sc1.enum1) +---- + +query-sql +SELECT currval('sc1.sq1') +---- +2 + +# Start a new cluster with the same IO dir. +new-cluster name=s2 share-io-dir=s1 +---- + +# Restore into the new cluster. +exec-sql cluster=s2 +RESTORE FROM LATEST IN 'nodelocal://1/test/' +---- + +exec-sql +USE db1 +---- + +# Make sure ids in signature and body are rewritten. +# 1. argument type id is rewritten so that type name is deserialized correctly. +# 2. db name in qualified name is rewritten. +# 3. sequence id is rewritten so that sequence name is deserialized correctly. +query-sql +SELECT create_statement FROM [SHOW CREATE PROCEDURE sc1.p1] +---- +---- +CREATE PROCEDURE sc1.p1(IN a db1.sc1.enum1) + LANGUAGE plpgsql + AS $$ + BEGIN + SELECT a FROM db1.sc1.tbl1; + SELECT nextval('sc1.sq1'::REGCLASS); + END + +$$ +---- +---- + +# Make sure procedure signature is rewritten in schema descriptor so that +# procedure can be resolved and executed. +exec-sql +CALL sc2.p2(456) +---- + +query-sql +SELECT * FROM sc2.tbl2 +---- +123 +456 + +# Make sure dependency IDs are rewritten. +# Note that technically this only tests forward-reference IDs in depended-on +# objects are rewritten. But since we have cross-references validation, so this +# also means back-references in the function descriptor are good. +exec-sql +DROP SEQUENCE sc1.sq1 +---- +pq: cannot drop sequence sq1 because other objects depend on it + +exec-sql +DROP TABLE sc1.tbl1 +---- +pq: cannot drop table tbl1 because other objects depend on it + +exec-sql +ALTER TABLE sc1.tbl1 RENAME TO tbl1_new +---- +pq: cannot rename relation "sc1.tbl1" because function "p1" depends on it +HINT: consider dropping "p1" first. + +exec-sql +ALTER TABLE sc1.tbl1 SET SCHEMA sc2; +---- +pq: cannot set schema on relation "tbl1" because function "p1" depends on it +HINT: consider dropping "p1" first. + +exec-sql +DROP TYPE sc1.enum1 +---- +pq: cannot drop type "enum1" because other objects ([db1.sc1.p1]) still depend on it + +# Make sure that backup and restore individual tables from schema with procedure +# does not crash. +new-cluster name=s3 +---- + +exec-sql cluster=s3 +CREATE DATABASE db1; +CREATE SCHEMA sc1; +CREATE TABLE sc1.t(a INT PRIMARY KEY); +CREATE PROCEDURE sc1.p() LANGUAGE PLpgSQL AS $$ BEGIN SELECT 1; END $$; +---- + +# Make sure the original schema has procedure signatures +query-sql +WITH db_id AS ( + SELECT id FROM system.namespace WHERE name = 'defaultdb' +), +schema_id AS ( + SELECT ns.id + FROM system.namespace AS ns + JOIN db_id ON ns."parentID" = db_id.id + WHERE ns.name = 'sc1' +) +SELECT id FROM schema_id; +---- +109 + +query-sql +WITH to_json AS ( + SELECT + id, + crdb_internal.pb_to_json( + 'cockroach.sql.sqlbase.Descriptor', + descriptor, + false + ) AS d + FROM + system.descriptor + WHERE id = 109 +) +SELECT d->'schema'->>'functions'::string FROM to_json; +---- +{"p": {"signatures": [{"id": 111, "isProcedure": true, "returnType": {"family": "VoidFamily", "oid": 2278}}]}} + +exec-sql +BACKUP TABLE sc1.t INTO 'nodelocal://1/test/' +---- + +exec-sql +RESTORE TABLE sc1.t FROM LATEST IN 'nodelocal://1/test/' WITH into_db = 'db1'; +---- + +exec-sql +USE db1; +---- + +query-sql +WITH db_id AS ( + SELECT id FROM system.namespace WHERE name = 'db1' +), +schema_id AS ( + SELECT ns.id + FROM system.namespace AS ns + JOIN db_id ON ns."parentID" = db_id.id + WHERE ns.name = 'sc1' +) +SELECT id FROM schema_id; +---- +112 + +query-sql +WITH to_json AS ( + SELECT + id, + crdb_internal.pb_to_json( + 'cockroach.sql.sqlbase.Descriptor', + descriptor, + false + ) AS d + FROM + system.descriptor + WHERE id = 112 +) +SELECT d->'schema'->>'functions'::string FROM to_json; +---- + + +# Make sure proper error message is returned when trying to resolve the +# procedure from the restore target db. +query-sql +CALL p() +---- +pq: procedure p does not exist diff --git a/pkg/ccl/backupccl/testdata/backup-restore/plpgsql_user_defined_functions b/pkg/ccl/backupccl/testdata/backup-restore/plpgsql_user_defined_functions new file mode 100644 index 000000000000..a3c80a7272bc --- /dev/null +++ b/pkg/ccl/backupccl/testdata/backup-restore/plpgsql_user_defined_functions @@ -0,0 +1,571 @@ +# Test backing up and restoring a database with PL/pgSQL user defined functions. +new-cluster name=s +---- + +exec-sql +CREATE DATABASE db1; +USE db1; +CREATE SCHEMA sc1; +CREATE TABLE sc1.tbl1(a INT PRIMARY KEY); +CREATE TYPE sc1.enum1 AS ENUM('Good'); +CREATE SEQUENCE sc1.sq1; +CREATE FUNCTION sc1.f1(a sc1.enum1) RETURNS INT LANGUAGE PLpgSQL AS $$ + DECLARE + x INT := 0; + BEGIN + SELECT a FROM sc1.tbl1; + RETURN nextval('sc1.sq1'); + END +$$; +CREATE SCHEMA sc2; +CREATE TABLE sc2.tbl2(a INT PRIMARY KEY); +CREATE FUNCTION sc2.f2() RETURNS INT LANGUAGE PLpgSQL AS $$ + DECLARE + x INT; + BEGIN + SELECT a INTO x FROM sc2.tbl2 LIMIT 1; + RETURN x; + END +$$; +---- + +exec-sql +INSERT INTO sc2.tbl2 VALUES (123) +---- + +query-sql +SELECT sc1.f1('Good'::sc1.enum1) +---- +1 + +query-sql +SELECT sc2.f2() +---- +123 + +exec-sql +BACKUP DATABASE db1 INTO 'nodelocal://1/test/' +---- + +query-sql +WITH descs AS ( + SHOW BACKUP LATEST IN 'nodelocal://1/test/' +) +SELECT database_name, parent_schema_name, object_name, object_type, is_full_cluster FROM descs +---- + db1 database false +db1 public schema false +db1 sc1 schema false +db1 sc1 tbl1 table false +db1 sc1 enum1 type false +db1 sc1 _enum1 type false +db1 sc1 sq1 table false +db1 sc1 f1 function false +db1 sc2 schema false +db1 sc2 tbl2 table false +db1 sc2 f2 function false + +query-sql +SELECT create_statement FROM [SHOW CREATE FUNCTION sc1.f1] +---- +---- +CREATE FUNCTION sc1.f1(IN a db1.sc1.enum1) + RETURNS INT8 + VOLATILE + NOT LEAKPROOF + CALLED ON NULL INPUT + LANGUAGE plpgsql + AS $$ + DECLARE + x INT8 := 0; + BEGIN + SELECT a FROM db1.sc1.tbl1; + RETURN nextval('sc1.sq1'::REGCLASS); + END + +$$ +---- +---- + +query-sql +SELECT sc1.f1('Good'::sc1.enum1) +---- +2 + +exec-sql +DROP DATABASE db1 +---- + +exec-sql +RESTORE DATABASE db1 FROM LATEST IN 'nodelocal://1/test/' WITH new_db_name = db1_new +---- + +exec-sql +USE db1_new +---- + +# Make sure ids in signature and body are rewritten. +# 1. argument type id is rewritten so that type name is deserialized correctly. +# 2. db name in qualified name is rewritten. +# 3. sequence id is rewritten so that sequence name is deserialized correctly. +query-sql +SELECT create_statement FROM [SHOW CREATE FUNCTION sc1.f1] +---- +---- +CREATE FUNCTION sc1.f1(IN a db1_new.sc1.enum1) + RETURNS INT8 + VOLATILE + NOT LEAKPROOF + CALLED ON NULL INPUT + LANGUAGE plpgsql + AS $$ + DECLARE + x INT8 := 0; + BEGIN + SELECT a FROM db1_new.sc1.tbl1; + RETURN nextval('sc1.sq1'::REGCLASS); + END + +$$ +---- +---- + +# Make sure function signature is rewritten in schema descriptor so that +# function can be resolved and executed. +query-sql +SELECT sc1.f1('Good'::db1_new.sc1.enum1) +---- +2 + +# Make sure function still queries from correct table. +query-sql +SELECT db1_new.sc2.f2() +---- +123 + +# Make sure dependency IDs are rewritten. +# Note that technically this only tests forward-reference IDs in depended-on +# objects are rewritten. But since we have cross-references validation, so this +# also means back-references in UDF descriptor are good. +exec-sql +DROP SEQUENCE sc1.sq1 +---- +pq: cannot drop sequence sq1 because other objects depend on it + +exec-sql +DROP TABLE sc1.tbl1 +---- +pq: cannot drop table tbl1 because other objects depend on it + +exec-sql +ALTER TABLE sc1.tbl1 RENAME TO tbl1_new +---- +pq: cannot rename relation "sc1.tbl1" because function "f1" depends on it +HINT: consider dropping "f1" first. + +exec-sql +ALTER TABLE sc1.tbl1 SET SCHEMA sc2; +---- +pq: cannot set schema on relation "tbl1" because function "f1" depends on it +HINT: consider dropping "f1" first. + +exec-sql +DROP TYPE sc1.enum1 +---- +pq: cannot drop type "enum1" because other objects ([db1_new.sc1.f1]) still depend on it + +# Test backing up and restoring a full cluster with user defined function. +new-cluster name=s1 +---- + +exec-sql cluster=s1 +CREATE DATABASE db1; +USE db1; +CREATE SCHEMA sc1; +CREATE TABLE sc1.tbl1(a INT PRIMARY KEY); +CREATE TYPE sc1.enum1 AS ENUM('Good'); +CREATE SEQUENCE sc1.sq1; +CREATE FUNCTION sc1.f1(a sc1.enum1) RETURNS INT LANGUAGE PLpgSQL AS $$ + DECLARE + x INT; + BEGIN + SELECT a FROM sc1.tbl1; + SELECT nextval('sc1.sq1') INTO x; + RETURN x; + END +$$; +CREATE SCHEMA sc2; +CREATE TABLE sc2.tbl2(a INT PRIMARY KEY); +CREATE FUNCTION sc2.f2() RETURNS INT LANGUAGE PLpgSQL AS $$ + BEGIN + RETURN (SELECT a FROM sc2.tbl2 LIMIT 1); + END +$$; +---- + +exec-sql +INSERT INTO sc2.tbl2 VALUES (123) +---- + +query-sql +SELECT sc1.f1('Good'::sc1.enum1) +---- +1 + +query-sql +SELECT sc2.f2() +---- +123 + +exec-sql +BACKUP INTO 'nodelocal://1/test/' +---- + +query-sql +WITH descs AS ( + SHOW BACKUP LATEST IN 'nodelocal://1/test/' +) +SELECT + database_name, parent_schema_name, object_name, object_type, is_full_cluster +FROM + descs +WHERE + database_name = 'db1' + +---- +db1 public schema true +db1 sc1 schema true +db1 sc1 tbl1 table true +db1 sc1 enum1 type true +db1 sc1 _enum1 type true +db1 sc1 sq1 table true +db1 sc1 f1 function true +db1 sc2 schema true +db1 sc2 tbl2 table true +db1 sc2 f2 function true + +query-sql +SELECT create_statement FROM [SHOW CREATE FUNCTION sc1.f1] +---- +---- +CREATE FUNCTION sc1.f1(IN a db1.sc1.enum1) + RETURNS INT8 + VOLATILE + NOT LEAKPROOF + CALLED ON NULL INPUT + LANGUAGE plpgsql + AS $$ + DECLARE + x INT8; + BEGIN + SELECT a FROM db1.sc1.tbl1; + SELECT nextval('sc1.sq1'::REGCLASS) INTO x; + RETURN x; + END + +$$ +---- +---- + +query-sql +SELECT sc1.f1('Good'::sc1.enum1) +---- +2 + +# Start a new cluster with the same IO dir. +new-cluster name=s2 share-io-dir=s1 +---- + +# Restore into the new cluster. +exec-sql cluster=s2 +RESTORE FROM LATEST IN 'nodelocal://1/test/' +---- + +exec-sql +USE db1 +---- + +# Make sure ids in signature and body are rewritten. +# 1. argument type id is rewritten so that type name is deserialized correctly. +# 2. db name in qualified name is rewritten. +# 3. sequence id is rewritten so that sequence name is deserialized correctly. +query-sql +SELECT create_statement FROM [SHOW CREATE FUNCTION sc1.f1] +---- +---- +CREATE FUNCTION sc1.f1(IN a db1.sc1.enum1) + RETURNS INT8 + VOLATILE + NOT LEAKPROOF + CALLED ON NULL INPUT + LANGUAGE plpgsql + AS $$ + DECLARE + x INT8; + BEGIN + SELECT a FROM db1.sc1.tbl1; + SELECT nextval('sc1.sq1'::REGCLASS) INTO x; + RETURN x; + END + +$$ +---- +---- + +# Make sure function signature is rewritten in schema descriptor so that +# function can be resolved and executed. +query-sql +SELECT sc1.f1('Good'::sc1.enum1) +---- +2 + +# Make sure function still queries from correct table. +query-sql +SELECT sc2.f2() +---- +123 + +# Make sure dependency IDs are rewritten. +# Note that technically this only tests forward-reference IDs in depended-on +# objects are rewritten. But since we have cross-references validation, so this +# also means back-references in UDF descriptor are good. +exec-sql +DROP SEQUENCE sc1.sq1 +---- +pq: cannot drop sequence sq1 because other objects depend on it + +exec-sql +DROP TABLE sc1.tbl1 +---- +pq: cannot drop table tbl1 because other objects depend on it + +exec-sql +ALTER TABLE sc1.tbl1 RENAME TO tbl1_new +---- +pq: cannot rename relation "sc1.tbl1" because function "f1" depends on it +HINT: consider dropping "f1" first. + +exec-sql +ALTER TABLE sc1.tbl1 SET SCHEMA sc2; +---- +pq: cannot set schema on relation "tbl1" because function "f1" depends on it +HINT: consider dropping "f1" first. + +exec-sql +DROP TYPE sc1.enum1 +---- +pq: cannot drop type "enum1" because other objects ([db1.sc1.f1]) still depend on it + +# Make sure that backup and restore individual tables from schema with UDF does +# not crash. +new-cluster name=s3 +---- + +exec-sql cluster=s3 +CREATE DATABASE db1; +CREATE SCHEMA sc1; +CREATE TABLE sc1.t(a INT PRIMARY KEY); +CREATE FUNCTION sc1.f() RETURNS INT LANGUAGE PLpgSQL AS $$ BEGIN RETURN 1; END $$; +---- + +# Make sure the original schema has function signatures +query-sql +WITH db_id AS ( + SELECT id FROM system.namespace WHERE name = 'defaultdb' +), +schema_id AS ( + SELECT ns.id + FROM system.namespace AS ns + JOIN db_id ON ns."parentID" = db_id.id + WHERE ns.name = 'sc1' +) +SELECT id FROM schema_id; +---- +109 + +query-sql +WITH to_json AS ( + SELECT + id, + crdb_internal.pb_to_json( + 'cockroach.sql.sqlbase.Descriptor', + descriptor, + false + ) AS d + FROM + system.descriptor + WHERE id = 109 +) +SELECT d->'schema'->>'functions'::string FROM to_json; +---- +{"f": {"signatures": [{"id": 111, "returnType": {"family": "IntFamily", "oid": 20, "width": 64}}]}} + +exec-sql +BACKUP TABLE sc1.t INTO 'nodelocal://1/test/' +---- + +exec-sql +RESTORE TABLE sc1.t FROM LATEST IN 'nodelocal://1/test/' WITH into_db = 'db1'; +---- + +exec-sql +USE db1; +---- + +query-sql +WITH db_id AS ( + SELECT id FROM system.namespace WHERE name = 'db1' +), +schema_id AS ( + SELECT ns.id + FROM system.namespace AS ns + JOIN db_id ON ns."parentID" = db_id.id + WHERE ns.name = 'sc1' +) +SELECT id FROM schema_id; +---- +112 + +query-sql +WITH to_json AS ( + SELECT + id, + crdb_internal.pb_to_json( + 'cockroach.sql.sqlbase.Descriptor', + descriptor, + false + ) AS d + FROM + system.descriptor + WHERE id = 112 +) +SELECT d->'schema'->>'functions'::string FROM to_json; +---- + + +# Make sure proper error message is returned when trying to resolve the +# function from the restore target db. +query-sql +SELECT f() +---- +pq: unknown function: f() + +# Test that backing up and restoring a cluster with a function on 23.2 does not +# grant EXECUTE privileges on the public role for functions where that privilege +# has been revoked. +new-cluster name=s4 +---- + +exec-sql cluster=s4 +CREATE DATABASE db1; +USE db1; +CREATE USER u1; +CREATE FUNCTION add(x INT, y INT) RETURNS INT LANGUAGE PLpgSQL AS 'BEGIN RETURN x + y; END'; +REVOKE EXECUTE ON FUNCTION ADD FROM public; +SET ROLE = u1; +---- + +query-sql +SELECT add(1, 2) +---- +pq: user u1 does not have EXECUTE privilege on function add + +query-sql +SELECT database_name, schema_name, routine_signature, grantee, privilege_type, is_grantable +FROM [SHOW GRANTS ON FUNCTION add] +---- +db1 public add(int8, int8) admin ALL true +db1 public add(int8, int8) root ALL true + +exec-sql +SET ROLE = root +---- + +exec-sql cluster=s4 +BACKUP INTO 'nodelocal://1/test/' +---- + +# Start a new cluster with the same IO dir. +new-cluster name=s5 share-io-dir=s4 +---- + +# Restore into the new cluster. +exec-sql cluster=s5 +RESTORE FROM LATEST IN 'nodelocal://1/test/' +---- + +exec-sql cluster=s5 +USE db1; +SET ROLE = u1; +---- + +query-sql cluster=s5 +SELECT add(1, 2) +---- +pq: user u1 does not have EXECUTE privilege on function add + +query-sql cluster=s5 +SELECT database_name, schema_name, routine_signature, grantee, privilege_type, is_grantable +FROM [SHOW GRANTS ON FUNCTION add] +---- +db1 public add(int8, int8) admin ALL true +db1 public add(int8, int8) root ALL true + +# Backing up and restoring a database with a function resets privileges on +# functions to the default privileges because we cannot be sure if the same +# users are present in the new cluster. +new-cluster name=s6 +---- + +exec-sql cluster=s6 +CREATE DATABASE db1; +USE db1; +CREATE USER u1; +CREATE FUNCTION add(x INT, y INT) RETURNS INT LANGUAGE PLpgSQL AS 'BEGIN RETURN x + y; END'; +REVOKE EXECUTE ON FUNCTION ADD FROM public; +SET ROLE = u1; +---- + +query-sql +SELECT add(1, 2) +---- +pq: user u1 does not have EXECUTE privilege on function add + +query-sql +SELECT database_name, schema_name, routine_signature, grantee, privilege_type, is_grantable +FROM [SHOW GRANTS ON FUNCTION add] +---- +db1 public add(int8, int8) admin ALL true +db1 public add(int8, int8) root ALL true + +exec-sql +SET ROLE = root +---- + +exec-sql cluster=s6 +BACKUP DATABASE db1 INTO 'nodelocal://1/test/' +---- + +# Restore into the new cluster. +exec-sql cluster=s6 +RESTORE DATABASE db1 FROM LATEST IN 'nodelocal://1/test/' WITH new_db_name = db1_new +---- + +exec-sql cluster=s6 +USE db1_new; +SET ROLE = u1; +---- + +# The user now has EXECUTE privilege via the public role. +query-sql cluster=s6 +SELECT add(1, 2) +---- +3 + +query-sql cluster=s6 +SELECT database_name, schema_name, routine_signature, grantee, privilege_type, is_grantable +FROM [SHOW GRANTS ON FUNCTION add] +---- +db1_new public add(int8, int8) admin ALL true +db1_new public add(int8, int8) public EXECUTE false +db1_new public add(int8, int8) root ALL true diff --git a/pkg/ccl/backupccl/testdata/backup-restore/procedures b/pkg/ccl/backupccl/testdata/backup-restore/procedures index d570adc2ad4e..58583a635ef6 100644 --- a/pkg/ccl/backupccl/testdata/backup-restore/procedures +++ b/pkg/ccl/backupccl/testdata/backup-restore/procedures @@ -20,6 +20,10 @@ CREATE PROCEDURE sc2.p2(i INT) LANGUAGE SQL AS $$ $$; ---- +exec-sql +CALL sc1.p1('Good'::sc1.enum1) +---- + exec-sql CALL sc2.p2(123) ---- @@ -68,7 +72,7 @@ CALL sc1.p1('Good'::sc1.enum1) query-sql SELECT currval('sc1.sq1') ---- -1 +2 exec-sql DROP DATABASE db1 @@ -82,7 +86,7 @@ exec-sql USE db1_new ---- -# Make sure procedure ids in signature and body are rewritten. +# Make sure ids in the signature and body are rewritten. # 1. argument type id is rewritten so that type name is deserialized correctly. # 2. db name in qualified name is rewritten. # 3. sequence id is rewritten so that sequence name is deserialized correctly. @@ -105,7 +109,7 @@ CALL sc1.p1('Good'::sc1.enum1) query-sql SELECT currval('sc1.sq1') ---- -1 +2 # Make sure procedure still inserts into the correct table. exec-sql @@ -173,6 +177,10 @@ CREATE PROCEDURE sc2.p2(i INT) LANGUAGE SQL AS $$ $$; ---- +exec-sql +CALL sc1.p1('Good'::sc1.enum1) +---- + exec-sql CALL sc2.p2(123) ---- @@ -226,7 +234,7 @@ CALL sc1.p1('Good'::sc1.enum1) query-sql SELECT currval('sc1.sq1') ---- -1 +2 # Start a new cluster with the same IO dir. new-cluster name=s2 share-io-dir=s1 @@ -241,7 +249,7 @@ exec-sql USE db1 ---- -# Make sure procedure ids in signature and body are rewritten. +# Make sure ids in signature and body are rewritten. # 1. argument type id is rewritten so that type name is deserialized correctly. # 2. db name in qualified name is rewritten. # 3. sequence id is rewritten so that sequence name is deserialized correctly. diff --git a/pkg/ccl/backupccl/testdata/backup-restore/user-defined-functions b/pkg/ccl/backupccl/testdata/backup-restore/user-defined-functions index bd77f22e60ca..8f6fd2562218 100644 --- a/pkg/ccl/backupccl/testdata/backup-restore/user-defined-functions +++ b/pkg/ccl/backupccl/testdata/backup-restore/user-defined-functions @@ -22,6 +22,11 @@ exec-sql INSERT INTO sc2.tbl2 VALUES (123) ---- +query-sql +SELECT sc1.f1('Good'::sc1.enum1) +---- +1 + query-sql SELECT sc2.f2() ---- @@ -66,7 +71,7 @@ $$ query-sql SELECT sc1.f1('Good'::sc1.enum1) ---- -1 +2 exec-sql DROP DATABASE db1 @@ -80,7 +85,7 @@ exec-sql USE db1_new ---- -# Make sure function ids in signature and body are rewritten. +# Make sure ids in signature and body are rewritten. # 1. argument type id is rewritten so that type name is deserialized correctly. # 2. db name in qualified name is rewritten. # 3. sequence id is rewritten so that sequence name is deserialized correctly. @@ -103,7 +108,7 @@ $$ query-sql SELECT sc1.f1('Good'::db1_new.sc1.enum1) ---- -1 +2 # Make sure function still queries from correct table. query-sql @@ -166,6 +171,11 @@ exec-sql INSERT INTO sc2.tbl2 VALUES (123) ---- +query-sql +SELECT sc1.f1('Good'::sc1.enum1) +---- +1 + query-sql SELECT sc2.f2() ---- @@ -215,7 +225,7 @@ $$ query-sql SELECT sc1.f1('Good'::sc1.enum1) ---- -1 +2 # Start a new cluster with the same IO dir. new-cluster name=s2 share-io-dir=s1 @@ -230,7 +240,7 @@ exec-sql USE db1 ---- -# Make sure function ids in signature and body are rewritten. +# Make sure ids in signature and body are rewritten. # 1. argument type id is rewritten so that type name is deserialized correctly. # 2. db name in qualified name is rewritten. # 3. sequence id is rewritten so that sequence name is deserialized correctly. @@ -253,7 +263,7 @@ $$ query-sql SELECT sc1.f1('Good'::sc1.enum1) ---- -1 +2 # Make sure function still queries from correct table. query-sql diff --git a/pkg/cmd/roachtest/tests/online_restore.go b/pkg/cmd/roachtest/tests/online_restore.go index 04fbf415239b..b156a0486204 100644 --- a/pkg/cmd/roachtest/tests/online_restore.go +++ b/pkg/cmd/roachtest/tests/online_restore.go @@ -56,101 +56,111 @@ func registerOnlineRestore(r registry.Registry) { }, } { for _, runOnline := range []bool{true, false} { - sp := sp - runOnline := runOnline + for _, runWorkload := range []bool{true, false} { + sp := sp + runOnline := runOnline + runWorkload := runWorkload - if runOnline { - sp.namePrefix = "online" - } else { - sp.namePrefix = "offline" - sp.skip = "used for ad hoc experiments" - } + if runOnline { + sp.namePrefix = "online/" + } else { + sp.namePrefix = "offline/" + sp.skip = "used for ad hoc experiments" + } + sp.namePrefix = sp.namePrefix + fmt.Sprintf("workload=%t", runWorkload) - sp.initTestName() - r.Add(registry.TestSpec{ - Name: sp.testName, - Owner: registry.OwnerDisasterRecovery, - Benchmark: true, - Cluster: sp.hardware.makeClusterSpecs(r, sp.backup.cloud), - Timeout: sp.timeout, - // These tests measure performance. To ensure consistent perf, - // disable metamorphic encryption. - EncryptionSupport: registry.EncryptionAlwaysDisabled, - CompatibleClouds: registry.Clouds(sp.backup.cloud), - Suites: sp.suites, - Skip: sp.skip, - Run: func(ctx context.Context, t test.Test, c cluster.Cluster) { + sp.initTestName() + r.Add(registry.TestSpec{ + Name: sp.testName, + Owner: registry.OwnerDisasterRecovery, + Benchmark: true, + Cluster: sp.hardware.makeClusterSpecs(r, sp.backup.cloud), + Timeout: sp.timeout, + // These tests measure performance. To ensure consistent perf, + // disable metamorphic encryption. + EncryptionSupport: registry.EncryptionAlwaysDisabled, + CompatibleClouds: registry.Clouds(sp.backup.cloud), + Suites: sp.suites, + Skip: sp.skip, + Run: func(ctx context.Context, t test.Test, c cluster.Cluster) { - testStartTime := timeutil.Now() + testStartTime := timeutil.Now() - rd := makeRestoreDriver(t, c, sp) - rd.prepareCluster(ctx) + rd := makeRestoreDriver(t, c, sp) + rd.prepareCluster(ctx) - m := c.NewMonitor(ctx, sp.hardware.getCRDBNodes()) - m.Go(func(ctx context.Context) error { - db, err := rd.c.ConnE(ctx, rd.t.L(), rd.c.Node(1)[0]) - if err != nil { - return err - } - defer db.Close() - if _, err := db.Exec("SET CLUSTER SETTING kv.queue.process.guaranteed_time_budget='1h'"); err != nil { - return err - } - if _, err := db.Exec("SET CLUSTER SETTING kv.snapshot_receiver.excise.enabled=true"); err != nil { - return err - } - if _, err := db.Exec("SET CLUSTER SETTING sql.stats.automatic_collection.enabled=false"); err != nil { - return err - } - opts := "" - if runOnline { - opts = "WITH EXPERIMENTAL DEFERRED COPY" - } - restoreCmd := rd.restoreCmd("DATABASE tpce", opts) - t.L().Printf("Running %s", restoreCmd) - if _, err = db.ExecContext(ctx, restoreCmd); err != nil { - return err - } - return nil - }) - m.Wait() + m := c.NewMonitor(ctx, sp.hardware.getCRDBNodes()) + m.Go(func(ctx context.Context) error { + db, err := rd.c.ConnE(ctx, rd.t.L(), rd.c.Node(1)[0]) + if err != nil { + return err + } + defer db.Close() + if _, err := db.Exec("SET CLUSTER SETTING kv.queue.process.guaranteed_time_budget='1h'"); err != nil { + return err + } + if _, err := db.Exec("SET CLUSTER SETTING kv.snapshot_receiver.excise.enabled=true"); err != nil { + return err + } + if _, err := db.Exec("SET CLUSTER SETTING sql.stats.automatic_collection.enabled=false"); err != nil { + return err + } + opts := "" + if runOnline { + opts = "WITH EXPERIMENTAL DEFERRED COPY" + } + restoreCmd := rd.restoreCmd("DATABASE tpce", opts) + t.L().Printf("Running %s", restoreCmd) + if _, err = db.ExecContext(ctx, restoreCmd); err != nil { + return err + } + return nil + }) + m.Wait() - workloadCtx, workloadCancel := context.WithCancel(ctx) - mDownload := c.NewMonitor(workloadCtx, sp.hardware.getCRDBNodes()) - // TODO(msbutler): add foreground query latency tracker + workloadCtx, workloadCancel := context.WithCancel(ctx) + mDownload := c.NewMonitor(workloadCtx, sp.hardware.getCRDBNodes()) + // TODO(msbutler): add foreground query latency tracker - mDownload.Go(func(ctx context.Context) error { - err := sp.backup.workload.run(ctx, t, c, sp.hardware) - // We expect the workload to return a context cancelled error because - // the roachtest driver cancels the monitor's context after the download job completes - if err != nil && ctx.Err() == nil { - // Implies the workload context was not cancelled and the workload cmd returned a - // different error. - return errors.Wrapf(err, `Workload context was not cancelled. Error returned by workload cmd`) - } - rd.t.L().Printf("workload successfully finished") - return nil - }) - mDownload.Go(func(ctx context.Context) error { - defer workloadCancel() - if runOnline { - return waitForDownloadJob(ctx, c, t.L()) - } - // If we just completed an offline restore and are running the - // workload, run the workload until we're at most 15 minutes - // away from timing out. - testRuntime := timeutil.Since(testStartTime) - workloadDuration := sp.timeout - testRuntime - if workloadDuration > time.Minute*15 { - workloadDuration = workloadDuration - time.Minute*15 - } - fmt.Printf("let workload run for %.2f minutes", workloadDuration.Minutes()) - time.Sleep(workloadDuration) - return nil - }) - mDownload.Wait() - }, - }) + mDownload.Go(func(ctx context.Context) error { + if !runWorkload { + fmt.Printf("roachtest configured to skip running the foreground workload") + return nil + } + err := sp.backup.workload.run(ctx, t, c, sp.hardware) + // We expect the workload to return a context cancelled error because + // the roachtest driver cancels the monitor's context after the download job completes + if err != nil && ctx.Err() == nil { + // Implies the workload context was not cancelled and the workload cmd returned a + // different error. + return errors.Wrapf(err, `Workload context was not cancelled. Error returned by workload cmd`) + } + rd.t.L().Printf("workload successfully finished") + return nil + }) + mDownload.Go(func(ctx context.Context) error { + defer workloadCancel() + if runOnline { + return waitForDownloadJob(ctx, c, t.L()) + } + if runWorkload { + // If we just completed an offline restore and are running the + // workload, run the workload until we're at most 15 minutes + // away from timing out. + testRuntime := timeutil.Since(testStartTime) + workloadDuration := sp.timeout - testRuntime + if workloadDuration > time.Minute*15 { + workloadDuration = workloadDuration - time.Minute*15 + } + fmt.Printf("let workload run for %.2f minutes", workloadDuration.Minutes()) + time.Sleep(workloadDuration) + } + return nil + }) + mDownload.Wait() + }, + }) + } } } } diff --git a/pkg/sql/catalog/redact/BUILD.bazel b/pkg/sql/catalog/redact/BUILD.bazel index a82f9afa8ff0..7a9f44977054 100644 --- a/pkg/sql/catalog/redact/BUILD.bazel +++ b/pkg/sql/catalog/redact/BUILD.bazel @@ -9,6 +9,7 @@ go_library( "//pkg/sql/catalog/catpb", "//pkg/sql/catalog/descpb", "//pkg/sql/parser", + "//pkg/sql/plpgsql/parser:plpgparser", "//pkg/sql/schemachanger/scpb", "//pkg/sql/sem/tree", "@com_github_cockroachdb_errors//:errors", @@ -24,6 +25,7 @@ go_test( deps = [ ":redact", "//pkg/base", + "//pkg/ccl", "//pkg/security/securityassets", "//pkg/security/securitytest", "//pkg/server", diff --git a/pkg/sql/catalog/redact/redact.go b/pkg/sql/catalog/redact/redact.go index 1ec0258f05c2..ebd407f8f928 100644 --- a/pkg/sql/catalog/redact/redact.go +++ b/pkg/sql/catalog/redact/redact.go @@ -16,6 +16,7 @@ import ( "github.com/cockroachdb/cockroach/pkg/sql/catalog/catpb" "github.com/cockroachdb/cockroach/pkg/sql/catalog/descpb" "github.com/cockroachdb/cockroach/pkg/sql/parser" + plpgsqlparser "github.com/cockroachdb/cockroach/pkg/sql/plpgsql/parser" "github.com/cockroachdb/cockroach/pkg/sql/schemachanger/scpb" "github.com/cockroachdb/cockroach/pkg/sql/sem/tree" "github.com/cockroachdb/errors" @@ -168,7 +169,7 @@ func redactElement(element scpb.Element) error { return redactExpr(&e.ComputeExpr.Expr) } case *scpb.FunctionBody: - return redactFunctionBodyStr(&e.Body) + return redactFunctionBodyStr(e.Lang.Lang, &e.Body) } return nil } @@ -217,7 +218,7 @@ func redactFunctionDescriptor(desc *descpb.FunctionDescriptor) (errs []error) { } } - if err := redactFunctionBodyStr(&desc.FunctionBody); err != nil { + if err := redactFunctionBodyStr(desc.Lang, &desc.FunctionBody); err != nil { return []error{err} } if scs := desc.DeclarativeSchemaChangerState; scs != nil { @@ -233,19 +234,29 @@ func redactFunctionDescriptor(desc *descpb.FunctionDescriptor) (errs []error) { return nil } -func redactFunctionBodyStr(body *string) error { - stmts, err := parser.Parse(*body) - if err != nil { - return err - } - +func redactFunctionBodyStr(lang catpb.Function_Language, body *string) error { fmtCtx := tree.NewFmtCtx(tree.FmtHideConstants) - for i, stmt := range stmts { - if i > 0 { - fmtCtx.WriteString(" ") + switch lang { + case catpb.Function_SQL: + stmts, err := parser.Parse(*body) + if err != nil { + return err + } + for i, stmt := range stmts { + if i > 0 { + fmtCtx.WriteString(" ") + } + fmtCtx.FormatNode(stmt.AST) + fmtCtx.WriteString(";") + } + case catpb.Function_PLPGSQL: + stmt, err := plpgsqlparser.Parse(*body) + if err != nil { + return err } fmtCtx.FormatNode(stmt.AST) - fmtCtx.WriteString(";") + default: + return errors.AssertionFailedf("unexpected function language %s", lang) } *body = fmtCtx.String() return nil diff --git a/pkg/sql/catalog/redact/redact_test.go b/pkg/sql/catalog/redact/redact_test.go index 5d73610d25fc..9ae8d696db12 100644 --- a/pkg/sql/catalog/redact/redact_test.go +++ b/pkg/sql/catalog/redact/redact_test.go @@ -15,6 +15,7 @@ import ( "testing" "github.com/cockroachdb/cockroach/pkg/base" + "github.com/cockroachdb/cockroach/pkg/ccl" "github.com/cockroachdb/cockroach/pkg/sql/catalog/desctestutils" "github.com/cockroachdb/cockroach/pkg/sql/catalog/funcdesc" "github.com/cockroachdb/cockroach/pkg/sql/catalog/redact" @@ -29,6 +30,7 @@ import ( func TestRedactQueries(t *testing.T) { defer leaktest.AfterTest(t)() defer log.Scope(t).Close(t) + defer ccl.TestingEnableEnterprise()() ctx := context.Background() srv, db, kvDB := serverutils.StartServer(t, base.TestServerArgs{}) @@ -39,11 +41,23 @@ func TestRedactQueries(t *testing.T) { tdb.Exec(t, "CREATE VIEW view AS SELECT k, v FROM kv WHERE v <> 'constant literal'") tdb.Exec(t, "CREATE TABLE ctas AS SELECT k, v FROM kv WHERE v <> 'constant literal'") tdb.Exec(t, ` -CREATE FUNCTION f1() RETURNS INT -LANGUAGE SQL -AS $$ +CREATE FUNCTION f1() RETURNS INT +LANGUAGE SQL +AS $$ SELECT k FROM kv WHERE v != 'foo'; SELECT k FROM kv WHERE v = 'bar'; +$$`) + tdb.Exec(t, ` +CREATE FUNCTION f2() RETURNS INT +LANGUAGE PLpgSQL +AS $$ +DECLARE +x INT := 0; +y TEXT := 'bar'; +BEGIN +SELECT k FROM kv WHERE v != 'foo'; +RETURN x + 3; +END $$`) t.Run("view", func(t *testing.T) { @@ -64,10 +78,24 @@ $$`) require.Equal(t, `SELECT k, v FROM defaultdb.public.kv WHERE v != '_'`, mut.CreateQuery) }) - t.Run("create function", func(t *testing.T) { + t.Run("create function sql", func(t *testing.T) { fn := desctestutils.TestingGetFunctionDescriptor(kvDB, codec, "defaultdb", "public", "f1") mut := funcdesc.NewBuilder(fn.FuncDesc()).BuildCreatedMutableFunction() require.Empty(t, redact.Redact(mut.DescriptorProto())) require.Equal(t, `SELECT k FROM defaultdb.public.kv WHERE v != '_'; SELECT k FROM defaultdb.public.kv WHERE v = '_';`, mut.FunctionBody) }) + + t.Run("create function plpgsql", func(t *testing.T) { + fn := desctestutils.TestingGetFunctionDescriptor(kvDB, codec, "defaultdb", "public", "f2") + mut := funcdesc.NewBuilder(fn.FuncDesc()).BuildCreatedMutableFunction() + require.Empty(t, redact.Redact(mut.DescriptorProto())) + require.Equal(t, `DECLARE +x INT8 := _; +y STRING := '_'; +BEGIN +SELECT k FROM defaultdb.public.kv WHERE v != '_'; +RETURN x + _; +END +`, mut.FunctionBody) + }) } diff --git a/pkg/sql/catalog/rewrite/BUILD.bazel b/pkg/sql/catalog/rewrite/BUILD.bazel index 96328501312d..9a2c5b311966 100644 --- a/pkg/sql/catalog/rewrite/BUILD.bazel +++ b/pkg/sql/catalog/rewrite/BUILD.bazel @@ -19,9 +19,12 @@ go_library( "//pkg/sql/parser", "//pkg/sql/pgwire/pgcode", "//pkg/sql/pgwire/pgerror", + "//pkg/sql/plpgsql/parser:plpgparser", "//pkg/sql/schemachanger/scpb", "//pkg/sql/schemachanger/screl", "//pkg/sql/sem/catid", + "//pkg/sql/sem/plpgsqltree", + "//pkg/sql/sem/plpgsqltree/utils", "//pkg/sql/sem/tree", "//pkg/sql/types", "//pkg/util/hlc", diff --git a/pkg/sql/catalog/rewrite/rewrite.go b/pkg/sql/catalog/rewrite/rewrite.go index 96c0b3ad92a0..c3f3fa89e55d 100644 --- a/pkg/sql/catalog/rewrite/rewrite.go +++ b/pkg/sql/catalog/rewrite/rewrite.go @@ -27,9 +27,12 @@ import ( "github.com/cockroachdb/cockroach/pkg/sql/parser" "github.com/cockroachdb/cockroach/pkg/sql/pgwire/pgcode" "github.com/cockroachdb/cockroach/pkg/sql/pgwire/pgerror" + plpgsqlparser "github.com/cockroachdb/cockroach/pkg/sql/plpgsql/parser" "github.com/cockroachdb/cockroach/pkg/sql/schemachanger/scpb" "github.com/cockroachdb/cockroach/pkg/sql/schemachanger/screl" "github.com/cockroachdb/cockroach/pkg/sql/sem/catid" + "github.com/cockroachdb/cockroach/pkg/sql/sem/plpgsqltree" + "github.com/cockroachdb/cockroach/pkg/sql/sem/plpgsqltree/utils" "github.com/cockroachdb/cockroach/pkg/sql/sem/tree" "github.com/cockroachdb/cockroach/pkg/sql/types" "github.com/cockroachdb/cockroach/pkg/util/hlc" @@ -323,27 +326,46 @@ func rewriteViewQueryDBNames(table *tabledesc.Mutable, newDB string) error { return nil } -func rewriteFunctionBodyDBNames(fnBody string, newDB string) (string, error) { - stmts, err := parser.Parse(fnBody) - if err != nil { - return "", err - } +func rewriteFunctionBodyDBNames( + fnBody string, newDB string, lang catpb.Function_Language, +) (string, error) { replaceFunc := makeDBNameReplaceFunc(newDB) + switch lang { + case catpb.Function_SQL: + fmtCtx := tree.NewFmtCtx(tree.FmtSimple) + stmts, err := parser.Parse(fnBody) + if err != nil { + return "", err + } + for i, stmt := range stmts { + if i > 0 { + fmtCtx.WriteString("\n") + } + f := tree.NewFmtCtx( + tree.FmtParsable, + tree.FmtReformatTableNames(replaceFunc), + ) + f.FormatNode(stmt.AST) + fmtCtx.WriteString(f.CloseAndGetString()) + fmtCtx.WriteString(";") + } + return fmtCtx.CloseAndGetString(), nil - fmtCtx := tree.NewFmtCtx(tree.FmtSimple) - for i, stmt := range stmts { - if i > 0 { - fmtCtx.WriteString("\n") + case catpb.Function_PLPGSQL: + stmt, err := plpgsqlparser.Parse(fnBody) + if err != nil { + return "", err } - f := tree.NewFmtCtx( + fmtCtx := tree.NewFmtCtx( tree.FmtParsable, tree.FmtReformatTableNames(replaceFunc), ) - f.FormatNode(stmt.AST) - fmtCtx.WriteString(f.CloseAndGetString()) - fmtCtx.WriteString(";") + fmtCtx.FormatNode(stmt.AST) + return fmtCtx.CloseAndGetString(), nil + + default: + return "", errors.AssertionFailedf("unexpected function language %s", lang) } - return fmtCtx.CloseAndGetString(), nil } // rewriteTypesInExpr rewrites all explicit ID type references in the input @@ -457,23 +479,40 @@ func rewriteSequencesInView(viewQuery string, rewrites jobspb.DescRewriteMap) (s return newStmt.String(), nil } -func rewriteSequencesInFunction(fnBody string, rewrites jobspb.DescRewriteMap) (string, error) { - stmts, err := parser.Parse(fnBody) - if err != nil { - return "", err - } - +func rewriteSequencesInFunction( + fnBody string, rewrites jobspb.DescRewriteMap, lang catpb.Function_Language, +) (string, error) { fmtCtx := tree.NewFmtCtx(tree.FmtSimple) - for i, stmt := range stmts { - newStmt, err := tree.SimpleStmtVisit(stmt.AST, makeSequenceReplaceFunc(rewrites)) + replaceSeqFunc := makeSequenceReplaceFunc(rewrites) + switch lang { + case catpb.Function_SQL: + stmts, err := parser.Parse(fnBody) if err != nil { return "", err } - if i > 0 { - fmtCtx.WriteString("\n") + for i, stmt := range stmts { + newStmt, err := tree.SimpleStmtVisit(stmt.AST, replaceSeqFunc) + if err != nil { + return "", err + } + if i > 0 { + fmtCtx.WriteString("\n") + } + fmtCtx.FormatNode(newStmt) + fmtCtx.WriteString(";") + } + + case catpb.Function_PLPGSQL: + stmt, err := plpgsqlparser.Parse(fnBody) + if err != nil { + return "", err } + v := utils.SQLStmtVisitor{Fn: replaceSeqFunc} + newStmt := plpgsqltree.Walk(&v, stmt.AST) fmtCtx.FormatNode(newStmt) - fmtCtx.WriteString(";") + + default: + return "", errors.AssertionFailedf("unexpected function language %s", lang) } return fmtCtx.CloseAndGetString(), nil } @@ -913,13 +952,13 @@ func FunctionDescs( // Rewrite function body. fnBody := fnDesc.FunctionBody if overrideDB != "" { - dbNameReplaced, err := rewriteFunctionBodyDBNames(fnDesc.FunctionBody, overrideDB) + dbNameReplaced, err := rewriteFunctionBodyDBNames(fnBody, overrideDB, fnDesc.Lang) if err != nil { return err } fnBody = dbNameReplaced } - fnBody, err := rewriteSequencesInFunction(fnBody, descriptorRewrites) + fnBody, err := rewriteSequencesInFunction(fnBody, descriptorRewrites, fnDesc.Lang) if err != nil { return err } diff --git a/pkg/sql/parser/parse_test.go b/pkg/sql/parser/parse_test.go index e06a68376e28..b665d3c17ec5 100644 --- a/pkg/sql/parser/parse_test.go +++ b/pkg/sql/parser/parse_test.go @@ -11,17 +11,14 @@ package parser_test import ( - "bytes" "fmt" "go/constant" "reflect" - "regexp" "strings" "testing" "github.com/cockroachdb/cockroach/pkg/sql/lexbase" "github.com/cockroachdb/cockroach/pkg/sql/parser" - "github.com/cockroachdb/cockroach/pkg/sql/pgwire/pgerror" "github.com/cockroachdb/cockroach/pkg/sql/randgen" _ "github.com/cockroachdb/cockroach/pkg/sql/sem/builtins" "github.com/cockroachdb/cockroach/pkg/sql/sem/tree" @@ -37,8 +34,6 @@ import ( "github.com/stretchr/testify/require" ) -var issueLinkRE = regexp.MustCompile("https://go.crdb.dev/issue-v/([0-9]+)/.*") - // TestParseDataDriven verifies that we can parse the supplied SQL and regenerate the SQL // string from the syntax tree. func TestParseDatadriven(t *testing.T) { @@ -46,67 +41,10 @@ func TestParseDatadriven(t *testing.T) { datadriven.RunTest(t, path, func(t *testing.T, d *datadriven.TestData) string { switch d.Cmd { case "parse": - // Check parse. - stmts, err := parser.Parse(d.Input) - if err != nil { - d.Fatalf(t, "unexpected parse error: %v", err) - } - - // Check pretty-print roundtrip via the sqlfmt logic. - sqlutils.VerifyStatementPrettyRoundtrip(t, d.Input) - - ref := stmts.String() - note := "" - if ref != d.Input { - note = " -- normalized!" - } - - // Check roundtrip and formatting with flags. - var buf bytes.Buffer - fmt.Fprintf(&buf, "%s%s\n", ref, note) - fmt.Fprintln(&buf, stmts.StringWithFlags(tree.FmtAlwaysGroupExprs), "-- fully parenthesized") - constantsHidden := stmts.StringWithFlags(tree.FmtHideConstants) - fmt.Fprintln(&buf, constantsHidden, "-- literals removed") - - // As of this writing, the SQL statement stats proceed as follows: - // first the literals are removed from statement to form a stat key, - // then the stat key is re-parsed, to undergo the anonymization stage. - // We also want to check the re-parsing is fine. - reparsedStmts, err := parser.Parse(constantsHidden) - if err != nil { - d.Fatalf(t, "unexpected error when reparsing without literals: %+v", err) - } else { - reparsedStmtsS := reparsedStmts.String() - if reparsedStmtsS != constantsHidden { - d.Fatalf(t, - "mismatched AST when reparsing without literals:\noriginal: %s\nexpected: %s\nactual: %s", - d.Input, constantsHidden, reparsedStmtsS, - ) - } - } - - fmt.Fprintln(&buf, stmts.StringWithFlags(tree.FmtAnonymize), "-- identifiers removed") - if strings.Contains(ref, tree.PasswordSubstitution) { - fmt.Fprintln(&buf, stmts.StringWithFlags(tree.FmtShowPasswords), "-- passwords exposed") - } - - return buf.String() - + return sqlutils.VerifyParseFormat(t, d.Input, false /* plpgsql */) case "error": _, err := parser.Parse(d.Input) - if err == nil { - return "" - } - pgerr := pgerror.Flatten(err) - msg := pgerr.Message - if pgerr.Detail != "" { - msg += "\nDETAIL: " + pgerr.Detail - } - if pgerr.Hint != "" { - msg += "\nHINT: " + pgerr.Hint - } - msg = issueLinkRE.ReplaceAllString(msg, "https://go.crdb.dev/issue-v/$1/") - return msg + return sqlutils.VerifyParseError(err) } d.Fatalf(t, "unsupported command: %s", d.Cmd) return "" diff --git a/pkg/sql/parser/statements/statement.go b/pkg/sql/parser/statements/statement.go index c4b4111bd0d8..160512ea9817 100644 --- a/pkg/sql/parser/statements/statement.go +++ b/pkg/sql/parser/statements/statement.go @@ -95,3 +95,11 @@ func (stmt PLpgStatement) StringWithFlags(flags tree.FmtFlags) string { stmt.AST.Format(ctx) return ctx.CloseAndGetString() } + +type ParsedStmts interface { + String() string + StringWithFlags(flags tree.FmtFlags) string +} + +var _ ParsedStmts = Statements{} +var _ ParsedStmts = PLpgStatement{} diff --git a/pkg/sql/plpgsql/parser/BUILD.bazel b/pkg/sql/plpgsql/parser/BUILD.bazel index c1a531ecb255..e6916aeb43c6 100644 --- a/pkg/sql/plpgsql/parser/BUILD.bazel +++ b/pkg/sql/plpgsql/parser/BUILD.bazel @@ -65,6 +65,7 @@ go_test( ":plpgparser", "//pkg/sql/sem/plpgsqltree/utils", "//pkg/testutils/datapathutils", + "//pkg/testutils/sqlutils", "@com_github_cockroachdb_datadriven//:datadriven", ], ) diff --git a/pkg/sql/plpgsql/parser/lexer.go b/pkg/sql/plpgsql/parser/lexer.go index 692008e30354..3fe7195e978d 100644 --- a/pkg/sql/plpgsql/parser/lexer.go +++ b/pkg/sql/plpgsql/parser/lexer.go @@ -489,3 +489,13 @@ func (l *lexer) ParseExpr(sqlStr string) (plpgsqltree.Expr, error) { } return exprs[0], nil } + +func checkLoopLabels(start, end string) error { + if start == "" && end != "" { + return errors.Newf("end label \"%s\" specified for unlabeled block", end) + } + if end != "" && start != end { + return errors.Newf("end label \"%s\" differs from block's label \"%s\"", end, start) + } + return nil +} diff --git a/pkg/sql/plpgsql/parser/parser_test.go b/pkg/sql/plpgsql/parser/parser_test.go index 191c8b7d9074..2a022d5095aa 100644 --- a/pkg/sql/plpgsql/parser/parser_test.go +++ b/pkg/sql/plpgsql/parser/parser_test.go @@ -13,32 +13,27 @@ package parser_test import ( "testing" - "github.com/cockroachdb/cockroach/pkg/sql/plpgsql/parser" + plpgsql "github.com/cockroachdb/cockroach/pkg/sql/plpgsql/parser" "github.com/cockroachdb/cockroach/pkg/sql/sem/plpgsqltree/utils" "github.com/cockroachdb/cockroach/pkg/testutils/datapathutils" + "github.com/cockroachdb/cockroach/pkg/testutils/sqlutils" "github.com/cockroachdb/datadriven" ) -// TODO: Define(if possible) a data driven test framework so that sql and -// plpgsql share a parse test func TestParseDataDriven(t *testing.T) { datadriven.Walk(t, datapathutils.TestDataPath(t), func(t *testing.T, path string) { datadriven.RunTest(t, path, func(t *testing.T, d *datadriven.TestData) string { switch d.Cmd { case "parse": - // Check parse. - fn, err := parser.Parse(d.Input) - if err != nil { - return err.Error() - } - // TODO(chengxiong): add pretty print round trip test. - return fn.String() + return sqlutils.VerifyParseFormat(t, d.Input, true /* plpgsql */) + case "error": + _, err := plpgsql.Parse(d.Input) + return sqlutils.VerifyParseError(err) case "feature-count": fn, err := utils.CountPLpgSQLStmt(d.Input) if err != nil { d.Fatalf(t, "unexpected parse error: %v", err) } - return fn.String() } d.Fatalf(t, "unsupported command: %s", d.Cmd) diff --git a/pkg/sql/plpgsql/parser/plpgsql.y b/pkg/sql/plpgsql/parser/plpgsql.y index 385f766e46b3..08aebae96375 100644 --- a/pkg/sql/plpgsql/parser/plpgsql.y +++ b/pkg/sql/plpgsql/parser/plpgsql.y @@ -952,6 +952,10 @@ opt_case_else: stmt_loop: opt_loop_label LOOP loop_body opt_label ';' { + loopLabel, loopEndLabel := $1, $4 + if err := checkLoopLabels(loopLabel, loopEndLabel); err != nil { + return setErr(plpgsqllex, err) + } $$.val = &plpgsqltree.Loop{ Label: $1, Body: $3.statements(), @@ -961,6 +965,10 @@ stmt_loop: opt_loop_label LOOP loop_body opt_label ';' stmt_while: opt_loop_label WHILE expr_until_loop LOOP loop_body opt_label ';' { + loopLabel, loopEndLabel := $1, $6 + if err := checkLoopLabels(loopLabel, loopEndLabel); err != nil { + return setErr(plpgsqllex, err) + } cond, err := plpgsqllex.(*lexer).ParseExpr($3) if err != nil { return setErr(plpgsqllex, err) @@ -1490,6 +1498,9 @@ opt_label: $$ = "" } | any_identifier + { + $$ = $1 + } ; opt_exitcond: ';' diff --git a/pkg/sql/plpgsql/parser/testdata/combo b/pkg/sql/plpgsql/parser/testdata/combo index 71678557c271..5cef6e41e9c0 100644 --- a/pkg/sql/plpgsql/parser/testdata/combo +++ b/pkg/sql/plpgsql/parser/testdata/combo @@ -36,3 +36,64 @@ END IF; END LOOP; RETURN res; END + -- normalized! +DECLARE +i INT8; +n INT8 := (array_length((arr), (1))); +res INT8[] := ((ARRAY[])::INT8[]); +BEGIN +i := (0); +LOOP +IF ((i) = (idx)) THEN + res := ((res) || (val)); +ELSE + res := ((res) || ((arr)[((i) + (1))])); +END IF; +i := ((i) + (1)); +IF ((i) >= (n)) THEN + EXIT; +END IF; +END LOOP; +RETURN (res); +END + -- fully parenthesized +DECLARE +i INT8; +n INT8 := array_length(arr, _); +res INT8[] := ARRAY[]::INT8[]; +BEGIN +i := _; +LOOP +IF i = idx THEN + res := res || val; +ELSE + res := res || arr[i + _]; +END IF; +i := i + _; +IF i >= n THEN + EXIT; +END IF; +END LOOP; +RETURN res; +END + -- literals removed +DECLARE +i INT8; +n INT8 := _(_, 1); +res INT8[] := ARRAY[]::INT8[]; +BEGIN +_ := 0; +LOOP +IF _ = _ THEN + _ := _ || _; +ELSE + _ := _ || _[_ + 1]; +END IF; +_ := _ + 1; +IF _ >= _ THEN + EXIT; +END IF; +END LOOP; +RETURN _; +END + -- identifiers removed diff --git a/pkg/sql/plpgsql/parser/testdata/decl_header b/pkg/sql/plpgsql/parser/testdata/decl_header index 97644cc9bf9c..22f2c2e9450b 100644 --- a/pkg/sql/plpgsql/parser/testdata/decl_header +++ b/pkg/sql/plpgsql/parser/testdata/decl_header @@ -8,6 +8,22 @@ DECLARE var1 INT8 := 30; BEGIN END + -- normalized! +DECLARE +var1 INT8 := (30); +BEGIN +END + -- fully parenthesized +DECLARE +var1 INT8 := _; +BEGIN +END + -- literals removed +DECLARE +var1 INT8 := 30; +BEGIN +END + -- identifiers removed parse DECLARE @@ -16,9 +32,25 @@ BEGIN END ---- DECLARE -var1 CONSTANT INT8 collation_name NOT NULL := 30; +var1 CONSTANT INT8 COLLATE collation_name NOT NULL := 30; +BEGIN +END + -- normalized! +DECLARE +var1 CONSTANT INT8 COLLATE collation_name NOT NULL := (30); +BEGIN +END + -- fully parenthesized +DECLARE +var1 CONSTANT INT8 COLLATE collation_name NOT NULL := _; BEGIN END + -- literals removed +DECLARE +var1 CONSTANT INT8 COLLATE _ NOT NULL := 30; +BEGIN +END + -- identifiers removed parse DECLARE @@ -27,18 +59,52 @@ BEGIN END ---- DECLARE -var1 CONSTANT INT8 collation_name NOT NULL := 30; +var1 CONSTANT INT8 COLLATE collation_name NOT NULL := 30; +BEGIN +END + -- normalized! +DECLARE +var1 CONSTANT INT8 COLLATE collation_name NOT NULL := (30); +BEGIN +END + -- fully parenthesized +DECLARE +var1 CONSTANT INT8 COLLATE collation_name NOT NULL := _; +BEGIN +END + -- literals removed +DECLARE +var1 CONSTANT INT8 COLLATE _ NOT NULL := 30; BEGIN END + -- identifiers removed -parse +error DECLARE var1 integer := 30; var2 ALIAS FOR quantity; BEGIN END ---- +---- at or near ";": syntax error: unimplemented: this syntax +DETAIL: source SQL: +DECLARE + var1 integer := 30; + var2 ALIAS FOR quantity; + ^ +HINT: You have attempted to use a feature that is not yet implemented. + +Please check the public issue tracker to check whether this problem is +already tracked. If you cannot find it there, please report the error +with details by creating a new issue. + +If you would rather not post publicly, please contact us directly +using the support form. + +We appreciate your feedback. +---- +---- parse DECLARE @@ -50,28 +116,79 @@ DECLARE var1 CURSOR FOR SELECT * FROM t1 WHERE id = arg1; BEGIN END + -- normalized! +DECLARE +var1 CURSOR FOR SELECT (*) FROM t1 WHERE ((id) = (arg1)); +BEGIN +END + -- fully parenthesized +DECLARE +var1 CURSOR FOR SELECT * FROM t1 WHERE id = arg1; +BEGIN +END + -- literals removed +DECLARE +var1 CURSOR FOR SELECT * FROM _ WHERE _ = _; +BEGIN +END + -- identifiers removed -parse +error DECLARE var1 NO SCROLL CURSOR (arg1 INTEGER) FOR SELECT * FROM t1 WHERE id = arg1; BEGIN END ---- +---- at or near "(": syntax error: unimplemented: this syntax +DETAIL: source SQL: +DECLARE + var1 NO SCROLL CURSOR (arg1 INTEGER) FOR SELECT * FROM t1 WHERE id = arg1; + ^ +HINT: You have attempted to use a feature that is not yet implemented. + +Please check the public issue tracker to check whether this problem is +already tracked. If you cannot find it there, please report the error +with details by creating a new issue. + +If you would rather not post publicly, please contact us directly +using the support form. + +We appreciate your feedback. +---- +---- # Correctly handle parsing errors for variable types. -parse +error DECLARE var1 one.two.three.four; BEGIN END ---- at or near "four": at or near ".": syntax error +DETAIL: source SQL: +SET ROW (1::one.two.three.four) + ^ +-- +source SQL: +DECLARE + var1 one.two.three.four; + ^ +HINT: try \h SET SESSION -parse +error DECLARE var1 one.two.three.four := 0; BEGIN END ---- at or near "four": at or near ".": syntax error +DETAIL: source SQL: +SET ROW (1::one.two.three.four ) + ^ +-- +source SQL: +DECLARE + var1 one.two.three.four := 0; + ^ +HINT: try \h SET SESSION diff --git a/pkg/sql/plpgsql/parser/testdata/exception_sect b/pkg/sql/plpgsql/parser/testdata/exception_sect index c14141f9ebf1..bd2712521cd4 100644 --- a/pkg/sql/plpgsql/parser/testdata/exception_sect +++ b/pkg/sql/plpgsql/parser/testdata/exception_sect @@ -5,7 +5,6 @@ BEGIN RETURN x; EXCEPTION WHEN division_by_zero THEN - ASSERT 0 == 0, 'error message'; RETURN 0; END; ---- @@ -15,9 +14,36 @@ x := 1; RETURN x; EXCEPTION WHEN division_by_zero THEN -ASSERT RETURN 0; END + -- normalized! +DECLARE +BEGIN +x := (1); +RETURN (x); +EXCEPTION +WHEN division_by_zero THEN +RETURN (0); +END + -- fully parenthesized +DECLARE +BEGIN +x := _; +RETURN x; +EXCEPTION +WHEN _ THEN +RETURN _; +END + -- literals removed +DECLARE +BEGIN +_ := 1; +RETURN _; +EXCEPTION +WHEN division_by_zero THEN +RETURN 0; +END + -- identifiers removed @@ -41,6 +67,37 @@ WHEN SQLSTATE '22012' THEN x := 22012; RETURN x; END + -- normalized! +DECLARE +BEGIN +x := (10); +RETURN (x); +EXCEPTION +WHEN SQLSTATE '22012' THEN +x := (22012); +RETURN (x); +END + -- fully parenthesized +DECLARE +BEGIN +x := _; +RETURN x; +EXCEPTION +WHEN SQLSTATE '_' THEN +x := _; +RETURN x; +END + -- literals removed +DECLARE +BEGIN +_ := 10; +RETURN _; +EXCEPTION +WHEN SQLSTATE '22012' THEN +_ := 22012; +RETURN _; +END + -- identifiers removed parse DECLARE @@ -62,6 +119,37 @@ WHEN SQLSTATE '22012' OR SQLSTATE '22005' THEN x := 100; RETURN x; END + -- normalized! +DECLARE +BEGIN +x := (10); +RETURN (x); +EXCEPTION +WHEN SQLSTATE '22012' OR SQLSTATE '22005' THEN +x := (100); +RETURN (x); +END + -- fully parenthesized +DECLARE +BEGIN +x := _; +RETURN x; +EXCEPTION +WHEN SQLSTATE '_' OR SQLSTATE '_' THEN +x := _; +RETURN x; +END + -- literals removed +DECLARE +BEGIN +_ := 10; +RETURN _; +EXCEPTION +WHEN SQLSTATE '22012' OR SQLSTATE '22005' THEN +_ := 100; +RETURN _; +END + -- identifiers removed parse DECLARE @@ -83,6 +171,37 @@ WHEN SQLSTATE '22012' OR SQLSTATE '22005' OR feature_not_supported THEN x := 100; RETURN x; END + -- normalized! +DECLARE +BEGIN +x := (10); +RETURN (x); +EXCEPTION +WHEN SQLSTATE '22012' OR SQLSTATE '22005' OR feature_not_supported THEN +x := (100); +RETURN (x); +END + -- fully parenthesized +DECLARE +BEGIN +x := _; +RETURN x; +EXCEPTION +WHEN SQLSTATE '_' OR SQLSTATE '_' OR _ THEN +x := _; +RETURN x; +END + -- literals removed +DECLARE +BEGIN +_ := 10; +RETURN _; +EXCEPTION +WHEN SQLSTATE '22012' OR SQLSTATE '22005' OR feature_not_supported THEN +_ := 100; +RETURN _; +END + -- identifiers removed parse DECLARE @@ -110,6 +229,46 @@ WHEN feature_not_supported THEN x := 12345; RETURN x; END + -- normalized! +DECLARE +BEGIN +x := (10); +RETURN (x); +EXCEPTION +WHEN SQLSTATE '22012' THEN +x := (22012); +RETURN (x); +WHEN feature_not_supported THEN +x := (12345); +RETURN (x); +END + -- fully parenthesized +DECLARE +BEGIN +x := _; +RETURN x; +EXCEPTION +WHEN SQLSTATE '_' THEN +x := _; +RETURN x; +WHEN _ THEN +x := _; +RETURN x; +END + -- literals removed +DECLARE +BEGIN +_ := 10; +RETURN _; +EXCEPTION +WHEN SQLSTATE '22012' THEN +_ := 22012; +RETURN _; +WHEN feature_not_supported THEN +_ := 12345; +RETURN _; +END + -- identifiers removed parse DECLARE @@ -137,3 +296,43 @@ WHEN feature_not_supported OR SQLSTATE '22005' THEN x := 12345; RETURN x; END + -- normalized! +DECLARE +BEGIN +x := (10); +RETURN (x); +EXCEPTION +WHEN SQLSTATE '22012' THEN +x := (22012); +RETURN (x); +WHEN feature_not_supported OR SQLSTATE '22005' THEN +x := (12345); +RETURN (x); +END + -- fully parenthesized +DECLARE +BEGIN +x := _; +RETURN x; +EXCEPTION +WHEN SQLSTATE '_' THEN +x := _; +RETURN x; +WHEN _ OR SQLSTATE '_' THEN +x := _; +RETURN x; +END + -- literals removed +DECLARE +BEGIN +_ := 10; +RETURN _; +EXCEPTION +WHEN SQLSTATE '22012' THEN +_ := 22012; +RETURN _; +WHEN feature_not_supported OR SQLSTATE '22005' THEN +_ := 12345; +RETURN _; +END + -- identifiers removed diff --git a/pkg/sql/plpgsql/parser/testdata/stmt_assert b/pkg/sql/plpgsql/parser/testdata/stmt_assert index 5f8f63a92055..ea7dacfbc1c8 100644 --- a/pkg/sql/plpgsql/parser/testdata/stmt_assert +++ b/pkg/sql/plpgsql/parser/testdata/stmt_assert @@ -1,12 +1,9 @@ -parse +feature-count DECLARE BEGIN ASSERT 1 > 2; ASSERT 1 > 2, 'error message' ; END ---- -DECLARE -BEGIN -ASSERT -ASSERT -END +stmt_assert: 2 +stmt_block: 1 diff --git a/pkg/sql/plpgsql/parser/testdata/stmt_assign b/pkg/sql/plpgsql/parser/testdata/stmt_assign index 29d37e8bbb7d..ff2e1a3b4cf3 100644 --- a/pkg/sql/plpgsql/parser/testdata/stmt_assign +++ b/pkg/sql/plpgsql/parser/testdata/stmt_assign @@ -10,6 +10,25 @@ BEGIN johnny := NULL; gyro := 7 + 7; END + -- normalized! +DECLARE +BEGIN +johnny := (NULL); +gyro := ((7) + (7)); +END + -- fully parenthesized +DECLARE +BEGIN +johnny := _; +gyro := _ + _; +END + -- literals removed +DECLARE +BEGIN +_ := NULL; +_ := 7 + 7; +END + -- identifiers removed parse DECLARE @@ -21,62 +40,115 @@ DECLARE BEGIN a := NULL; END + -- normalized! +DECLARE +BEGIN +a := (NULL); +END + -- fully parenthesized +DECLARE +BEGIN +a := _; +END + -- literals removed +DECLARE +BEGIN +_ := NULL; +END + -- identifiers removed -parse +error DECLARE BEGIN a :=; END ---- at or near ":": syntax error: missing expression +DETAIL: source SQL: +DECLARE +BEGIN + a :=; + ^ -parse +error DECLARE BEGIN johnny := (NULL; END ---- at or near "EOF": syntax error: mismatched parentheses +DETAIL: source SQL: +DECLARE +BEGIN +johnny := (NULL; +END + ^ -parse +error DECLARE BEGIN johnny := NULL); END ---- at or near "null": syntax error: mismatched parentheses +DETAIL: source SQL: +DECLARE +BEGIN +johnny := NULL); + ^ -parse +error DECLARE BEGIN johnny := (1 + (2); END ---- at or near "EOF": syntax error: mismatched parentheses +DETAIL: source SQL: +DECLARE +BEGIN +johnny := (1 + (2); +END + ^ -parse +error DECLARE BEGIN a := 1, 'string'; END ---- at or near ";": syntax error: query returned 2 columns +DETAIL: source SQL: +DECLARE +BEGIN + a := 1, 'string'; + ^ -parse +error DECLARE BEGIN a := 1, (2, 3, 4, 5); END ---- at or near ";": syntax error: query returned 2 columns +DETAIL: source SQL: +DECLARE +BEGIN + a := 1, (2, 3, 4, 5); + ^ -parse +error DECLARE BEGIN a := 1, (2, 3, 4, 5), 'abcd', true, ((1)); END ---- at or near ";": syntax error: query returned 5 columns +DETAIL: source SQL: +DECLARE +BEGIN + a := 1, (2, 3, 4, 5), 'abcd', true, ((1)); + ^ feature-count DECLARE diff --git a/pkg/sql/plpgsql/parser/testdata/stmt_block b/pkg/sql/plpgsql/parser/testdata/stmt_block index edf3ce6bf59d..0fb06f7195ee 100644 --- a/pkg/sql/plpgsql/parser/testdata/stmt_block +++ b/pkg/sql/plpgsql/parser/testdata/stmt_block @@ -6,3 +6,16 @@ END DECLARE BEGIN END + -- normalized! +DECLARE +BEGIN +END + -- fully parenthesized +DECLARE +BEGIN +END + -- literals removed +DECLARE +BEGIN +END + -- identifiers removed diff --git a/pkg/sql/plpgsql/parser/testdata/stmt_call b/pkg/sql/plpgsql/parser/testdata/stmt_call index 784921be8b99..acd21f45a516 100644 --- a/pkg/sql/plpgsql/parser/testdata/stmt_call +++ b/pkg/sql/plpgsql/parser/testdata/stmt_call @@ -1,12 +1,9 @@ -parse +feature-count DECLARE BEGIN CALL fn(1); DO $$ this is a code block $$; END ---- -DECLARE -BEGIN -CALL a function/procedure -DO a code block -END +stmt_block: 1 +stmt_call: 2 diff --git a/pkg/sql/plpgsql/parser/testdata/stmt_case b/pkg/sql/plpgsql/parser/testdata/stmt_case index adfd3d304de1..ddf5c7c1f5a0 100644 --- a/pkg/sql/plpgsql/parser/testdata/stmt_case +++ b/pkg/sql/plpgsql/parser/testdata/stmt_case @@ -1,4 +1,4 @@ -parse +feature-count DECLARE BEGIN CASE hello @@ -6,18 +6,11 @@ WHEN world THEN END CASE; END ---- ----- -DECLARE -BEGIN -CASE hello - -WHEN world THEN -END CASE -END ----- ----- +stmt_block: 1 +stmt_case: 1 +stmt_when: 1 -parse +feature-count DECLARE BEGIN CASE order_cnt @@ -25,18 +18,11 @@ WHEN 1, 2, 3 THEN END CASE; END ---- ----- -DECLARE -BEGIN -CASE order_cnt - -WHEN 1, 2, 3 THEN -END CASE -END ----- ----- +stmt_block: 1 +stmt_case: 1 +stmt_when: 1 -parse +feature-count DECLARE BEGIN CASE order_cnt @@ -45,19 +31,11 @@ WHEN 5 THEN END CASE; END ---- ----- -DECLARE -BEGIN -CASE order_cnt - -WHEN 1, 2, 3 THEN -WHEN 5 THEN -END CASE -END ----- ----- +stmt_block: 1 +stmt_case: 1 +stmt_when: 2 -parse +feature-count DECLARE BEGIN CASE @@ -65,14 +43,11 @@ WHEN true THEN END CASE; END ---- -DECLARE -BEGIN -CASE -WHEN true THEN -END CASE -END +stmt_block: 1 +stmt_case: 1 +stmt_when: 1 -parse +feature-count DECLARE order_cnt integer := 10; BEGIN @@ -82,16 +57,12 @@ WHEN order_cnt > 100 THEN END CASE; END ---- -DECLARE -order_cnt INT8 := 10; -BEGIN -CASE -WHEN order_cnt BETWEEN 0 AND 100 THEN -WHEN order_cnt > 100 THEN -END CASE -END +decl_stmt: 1 +stmt_block: 1 +stmt_case: 1 +stmt_when: 2 -parse +feature-count DECLARE order_cnt integer := 10; BEGIN @@ -105,18 +76,11 @@ BEGIN END CASE; END ---- -DECLARE -order_cnt INT8 := 10; -BEGIN -CASE -WHEN order_cnt BETWEEN 0 AND 100 THEN - CALL a function/procedure -WHEN order_cnt > 100 THEN - CALL a function/procedure -ELSE - CALL a function/procedure -END CASE -END +decl_stmt: 1 +stmt_block: 1 +stmt_call: 3 +stmt_case: 1 +stmt_when: 2 feature-count diff --git a/pkg/sql/plpgsql/parser/testdata/stmt_close b/pkg/sql/plpgsql/parser/testdata/stmt_close index 90edb47193a7..02479769279a 100644 --- a/pkg/sql/plpgsql/parser/testdata/stmt_close +++ b/pkg/sql/plpgsql/parser/testdata/stmt_close @@ -8,3 +8,19 @@ DECLARE BEGIN CLOSE some_cursor; END + -- normalized! +DECLARE +BEGIN +CLOSE some_cursor; +END + -- fully parenthesized +DECLARE +BEGIN +CLOSE some_cursor; +END + -- literals removed +DECLARE +BEGIN +CLOSE _; +END + -- identifiers removed diff --git a/pkg/sql/plpgsql/parser/testdata/stmt_commit_rollback b/pkg/sql/plpgsql/parser/testdata/stmt_commit_rollback index 002f07b83ea6..df50a0cf887b 100644 --- a/pkg/sql/plpgsql/parser/testdata/stmt_commit_rollback +++ b/pkg/sql/plpgsql/parser/testdata/stmt_commit_rollback @@ -1,4 +1,4 @@ -parse +error DECLARE BEGIN IF x THEN @@ -6,9 +6,28 @@ IF x THEN END IF; END ---- +---- at or near ";": syntax error: unimplemented: this syntax +DETAIL: source SQL: +DECLARE +BEGIN +IF x THEN + COMMIT; + ^ +HINT: You have attempted to use a feature that is not yet implemented. + +Please check the public issue tracker to check whether this problem is +already tracked. If you cannot find it there, please report the error +with details by creating a new issue. + +If you would rather not post publicly, please contact us directly +using the support form. + +We appreciate your feedback. +---- +---- -parse +error DECLARE BEGIN IF x THEN @@ -16,9 +35,28 @@ IF x THEN END IF; END ---- +---- at or near ";": syntax error: unimplemented: this syntax +DETAIL: source SQL: +DECLARE +BEGIN +IF x THEN + ROLLBACK; + ^ +HINT: You have attempted to use a feature that is not yet implemented. + +Please check the public issue tracker to check whether this problem is +already tracked. If you cannot find it there, please report the error +with details by creating a new issue. + +If you would rather not post publicly, please contact us directly +using the support form. + +We appreciate your feedback. +---- +---- -parse +error DECLARE BEGIN IF x THEN @@ -28,14 +66,52 @@ ELSIF y THEN END IF; END ---- +---- at or near ";": syntax error: unimplemented: this syntax +DETAIL: source SQL: +DECLARE +BEGIN +IF x THEN + COMMIT; + ^ +HINT: You have attempted to use a feature that is not yet implemented. + +Please check the public issue tracker to check whether this problem is +already tracked. If you cannot find it there, please report the error +with details by creating a new issue. + +If you would rather not post publicly, please contact us directly +using the support form. + +We appreciate your feedback. +---- +---- -parse +error DECLARE BEGIN INSERT INTO t1 VALUES (1,2) RETURNING x INTO y; COMMIT; END ---- +---- at or near ";": syntax error: unimplemented: this syntax +DETAIL: source SQL: +DECLARE +BEGIN + INSERT INTO t1 VALUES (1,2) RETURNING x INTO y; + COMMIT; + ^ +HINT: You have attempted to use a feature that is not yet implemented. + +Please check the public issue tracker to check whether this problem is +already tracked. If you cannot find it there, please report the error +with details by creating a new issue. + +If you would rather not post publicly, please contact us directly +using the support form. + +We appreciate your feedback. +---- +---- diff --git a/pkg/sql/plpgsql/parser/testdata/stmt_dynexec b/pkg/sql/plpgsql/parser/testdata/stmt_dynexec index dfc189d41ed8..58b9f1ad3ed2 100644 --- a/pkg/sql/plpgsql/parser/testdata/stmt_dynexec +++ b/pkg/sql/plpgsql/parser/testdata/stmt_dynexec @@ -1,54 +1,44 @@ -parse +feature-count DECLARE BEGIN EXECUTE 'any command'; END ---- -DECLARE -BEGIN -EXECUTE a dynamic command -END +stmt_block: 1 +stmt_dyn_exec: 1 -parse +feature-count DECLARE BEGIN EXECUTE 'any command' INTO x1; END ---- -DECLARE -BEGIN -EXECUTE a dynamic command WITH INTO -END +stmt_block: 1 +stmt_dyn_exec: 1 -parse +feature-count DECLARE BEGIN EXECUTE 'any command' INTO STRICT x1; END ---- -DECLARE -BEGIN -EXECUTE a dynamic command WITH INTO STRICT -END +stmt_block: 1 +stmt_dyn_exec: 1 -parse +feature-count DECLARE BEGIN EXECUTE 'any command' INTO x1 USING x2; END ---- -DECLARE -BEGIN -EXECUTE a dynamic command WITH INTO WITH USING -END +stmt_block: 1 +stmt_dyn_exec: 1 -parse +feature-count DECLARE BEGIN EXECUTE 'any command' INTO x1, x2 USING y1, y2; END ---- -DECLARE -BEGIN -EXECUTE a dynamic command WITH INTO WITH USING -END +stmt_block: 1 +stmt_dyn_exec: 1 diff --git a/pkg/sql/plpgsql/parser/testdata/stmt_exec_sql b/pkg/sql/plpgsql/parser/testdata/stmt_exec_sql index adeb389faa33..7bb30fca6e66 100644 --- a/pkg/sql/plpgsql/parser/testdata/stmt_exec_sql +++ b/pkg/sql/plpgsql/parser/testdata/stmt_exec_sql @@ -10,6 +10,22 @@ DECLARE BEGIN IMPORT TABLE foo FROM PGDUMP 'userfile://defaultdb.public.userfiles_root/db.sql' WITH OPTIONS (max_row_size = '524288'); END + -- normalized! +DECLARE +BEGIN +IMPORT TABLE foo FROM PGDUMP ('userfile://defaultdb.public.userfiles_root/db.sql') WITH OPTIONS (max_row_size = ('524288')); +END + -- fully parenthesized +DECLARE +BEGIN +IMPORT TABLE foo FROM PGDUMP '_' WITH OPTIONS (max_row_size = '_'); +END + -- literals removed +DECLARE +BEGIN +IMPORT TABLE _ FROM PGDUMP 'userfile://defaultdb.public.userfiles_root/db.sql' WITH OPTIONS (_ = '524288'); +END + -- identifiers removed parse DECLARE @@ -21,6 +37,22 @@ DECLARE BEGIN INSERT INTO t1 VALUES (1, 2); END + -- normalized! +DECLARE +BEGIN +INSERT INTO t1 VALUES ((1), (2)); +END + -- fully parenthesized +DECLARE +BEGIN +INSERT INTO t1 VALUES (_, _); +END + -- literals removed +DECLARE +BEGIN +INSERT INTO _ VALUES (1, 2); +END + -- identifiers removed parse DECLARE @@ -32,6 +64,22 @@ DECLARE BEGIN INSERT INTO t1 VALUES (1, 2) RETURNING x INTO y; END + -- normalized! +DECLARE +BEGIN +INSERT INTO t1 VALUES ((1), (2)) RETURNING (x) INTO y; +END + -- fully parenthesized +DECLARE +BEGIN +INSERT INTO t1 VALUES (_, _) RETURNING x INTO y; +END + -- literals removed +DECLARE +BEGIN +INSERT INTO _ VALUES (1, 2) RETURNING _ INTO _; +END + -- identifiers removed parse DECLARE @@ -43,14 +91,35 @@ DECLARE BEGIN INSERT INTO t1 VALUES (1, 2) RETURNING x INTO STRICT y; END + -- normalized! +DECLARE +BEGIN +INSERT INTO t1 VALUES ((1), (2)) RETURNING (x) INTO STRICT y; +END + -- fully parenthesized +DECLARE +BEGIN +INSERT INTO t1 VALUES (_, _) RETURNING x INTO STRICT y; +END + -- literals removed +DECLARE +BEGIN +INSERT INTO _ VALUES (1, 2) RETURNING _ INTO STRICT _; +END + -- identifiers removed -parse +error DECLARE BEGIN INSERT INTO t1 VALUES (1,2) INTO y; END ---- at or near ";": syntax error: INTO used with a command that cannot return data +DETAIL: source SQL: +DECLARE +BEGIN + INSERT INTO t1 VALUES (1,2) INTO y; + ^ parse DECLARE @@ -62,6 +131,22 @@ DECLARE BEGIN IMPORT INTO foo(k, v) CSV DATA ($1, $2); END + -- normalized! +DECLARE +BEGIN +IMPORT INTO foo(k, v) CSV DATA (($1), ($2)); +END + -- fully parenthesized +DECLARE +BEGIN +IMPORT INTO foo(k, v) CSV DATA ($1, $1); +END + -- literals removed +DECLARE +BEGIN +IMPORT INTO _(_, _) CSV DATA ($1, $2); +END + -- identifiers removed parse DECLARE @@ -73,6 +158,22 @@ DECLARE BEGIN SELECT x, y FROM xy; END + -- normalized! +DECLARE +BEGIN +SELECT (x), (y) FROM xy; +END + -- fully parenthesized +DECLARE +BEGIN +SELECT x, y FROM xy; +END + -- literals removed +DECLARE +BEGIN +SELECT _, _ FROM _; +END + -- identifiers removed parse DECLARE @@ -84,6 +185,22 @@ DECLARE BEGIN SELECT x, y FROM xy INTO a, b; END + -- normalized! +DECLARE +BEGIN +SELECT (x), (y) FROM xy INTO a, b; +END + -- fully parenthesized +DECLARE +BEGIN +SELECT x, y FROM xy INTO a, b; +END + -- literals removed +DECLARE +BEGIN +SELECT _, _ FROM _ INTO _, _; +END + -- identifiers removed parse DECLARE @@ -95,6 +212,22 @@ DECLARE BEGIN SELECT x, y FROM xy INTO a, b; END + -- normalized! +DECLARE +BEGIN +SELECT (x), (y) FROM xy INTO a, b; +END + -- fully parenthesized +DECLARE +BEGIN +SELECT x, y FROM xy INTO a, b; +END + -- literals removed +DECLARE +BEGIN +SELECT _, _ FROM _ INTO _, _; +END + -- identifiers removed parse DECLARE @@ -106,6 +239,22 @@ DECLARE BEGIN SET testing_optimizer_disable_rule_probability = 0; END + -- normalized! +DECLARE +BEGIN +SET testing_optimizer_disable_rule_probability = (0); +END + -- fully parenthesized +DECLARE +BEGIN +SET testing_optimizer_disable_rule_probability = _; +END + -- literals removed +DECLARE +BEGIN +SET testing_optimizer_disable_rule_probability = 0; +END + -- identifiers removed parse DECLARE @@ -131,30 +280,84 @@ SELECT max(x) FROM xy INTO i; INSERT INTO xy VALUES (10, 10) RETURNING x INTO i; RETURN i; END + -- normalized! +DECLARE +i INT8; +BEGIN +SET testing_optimizer_disable_rule_probability = (0); +INSERT INTO xy VALUES ((1), (2)); +SELECT ((1) + (1)); +SELECT (100) INTO i; +SELECT (max((x))) FROM xy INTO i; +INSERT INTO xy VALUES ((10), (10)) RETURNING (x) INTO i; +RETURN (i); +END + -- fully parenthesized +DECLARE +i INT8; +BEGIN +SET testing_optimizer_disable_rule_probability = _; +INSERT INTO xy VALUES (_, _); +SELECT _ + _; +SELECT _ INTO i; +SELECT max(x) FROM xy INTO i; +INSERT INTO xy VALUES (_, _) RETURNING x INTO i; +RETURN i; +END + -- literals removed +DECLARE +i INT8; +BEGIN +SET testing_optimizer_disable_rule_probability = 0; +INSERT INTO _ VALUES (1, 2); +SELECT 1 + 1; +SELECT 100 INTO _; +SELECT _(_) FROM _ INTO _; +INSERT INTO _ VALUES (10, 10) RETURNING _ INTO _; +RETURN _; +END + -- identifiers removed -parse +error DECLARE BEGIN INSERT INTO t1 VALUES 1,2); END ---- at or near "2": syntax error: mismatched parentheses +DETAIL: source SQL: +DECLARE +BEGIN + INSERT INTO t1 VALUES 1,2); + ^ -parse +error DECLARE BEGIN INSERT INTO t1 VALUES (1,2; END ---- at or near "EOF": syntax error: mismatched parentheses +DETAIL: source SQL: +DECLARE +BEGIN + INSERT INTO t1 VALUES (1,2; +END + ^ -parse +error DECLARE BEGIN INSERT INTO t1 (VALUES (1,2); END ---- at or near "EOF": syntax error: mismatched parentheses +DETAIL: source SQL: +DECLARE +BEGIN + INSERT INTO t1 (VALUES (1,2); +END + ^ # Regression test for #112940 - UPSERT INTO should be allowed. parse @@ -167,6 +370,22 @@ DECLARE BEGIN UPSERT INTO t1 VALUES (1, 2) RETURNING x INTO y; END + -- normalized! +DECLARE +BEGIN +UPSERT INTO t1 VALUES ((1), (2)) RETURNING (x) INTO y; +END + -- fully parenthesized +DECLARE +BEGIN +UPSERT INTO t1 VALUES (_, _) RETURNING x INTO y; +END + -- literals removed +DECLARE +BEGIN +UPSERT INTO _ VALUES (1, 2) RETURNING _ INTO _; +END + -- identifiers removed parse DECLARE @@ -178,11 +397,32 @@ DECLARE BEGIN UPSERT INTO t1 VALUES (1, 2) RETURNING x INTO STRICT y; END + -- normalized! +DECLARE +BEGIN +UPSERT INTO t1 VALUES ((1), (2)) RETURNING (x) INTO STRICT y; +END + -- fully parenthesized +DECLARE +BEGIN +UPSERT INTO t1 VALUES (_, _) RETURNING x INTO STRICT y; +END + -- literals removed +DECLARE +BEGIN +UPSERT INTO _ VALUES (1, 2) RETURNING _ INTO STRICT _; +END + -- identifiers removed -parse +error DECLARE BEGIN UPSERT INTO t1 VALUES (1,2) INTO y; END ---- at or near ";": syntax error: INTO used with a command that cannot return data +DETAIL: source SQL: +DECLARE +BEGIN + UPSERT INTO t1 VALUES (1,2) INTO y; + ^ diff --git a/pkg/sql/plpgsql/parser/testdata/stmt_exit b/pkg/sql/plpgsql/parser/testdata/stmt_exit index 7e8b1a7e4073..fe6d8931a45d 100644 --- a/pkg/sql/plpgsql/parser/testdata/stmt_exit +++ b/pkg/sql/plpgsql/parser/testdata/stmt_exit @@ -10,6 +10,25 @@ BEGIN EXIT some_label; EXIT some_label WHEN some_condition; END + -- normalized! +DECLARE +BEGIN +EXIT some_label; +EXIT some_label WHEN (some_condition); +END + -- fully parenthesized +DECLARE +BEGIN +EXIT some_label; +EXIT some_label WHEN some_condition; +END + -- literals removed +DECLARE +BEGIN +EXIT _; +EXIT _ WHEN _; +END + -- identifiers removed parse DECLARE @@ -23,3 +42,22 @@ BEGIN CONTINUE some_label; CONTINUE some_label WHEN some_condition; END + -- normalized! +DECLARE +BEGIN +CONTINUE some_label; +CONTINUE some_label WHEN (some_condition); +END + -- fully parenthesized +DECLARE +BEGIN +CONTINUE some_label; +CONTINUE some_label WHEN some_condition; +END + -- literals removed +DECLARE +BEGIN +CONTINUE _; +CONTINUE _ WHEN _; +END + -- identifiers removed diff --git a/pkg/sql/plpgsql/parser/testdata/stmt_fetch_move b/pkg/sql/plpgsql/parser/testdata/stmt_fetch_move index 9b65d5cea7d8..0d101825b370 100644 --- a/pkg/sql/plpgsql/parser/testdata/stmt_fetch_move +++ b/pkg/sql/plpgsql/parser/testdata/stmt_fetch_move @@ -8,6 +8,22 @@ DECLARE BEGIN MOVE 1 FROM emp_cur; END + -- normalized! +DECLARE +BEGIN +MOVE 1 FROM emp_cur; +END + -- fully parenthesized +DECLARE +BEGIN +MOVE 1 FROM emp_cur; +END + -- literals removed +DECLARE +BEGIN +MOVE 1 FROM _; +END + -- identifiers removed parse DECLARE @@ -19,6 +35,22 @@ DECLARE BEGIN MOVE -1 FROM var; END + -- normalized! +DECLARE +BEGIN +MOVE -1 FROM var; +END + -- fully parenthesized +DECLARE +BEGIN +MOVE -1 FROM var; +END + -- literals removed +DECLARE +BEGIN +MOVE -1 FROM _; +END + -- identifiers removed parse DECLARE @@ -30,6 +62,22 @@ DECLARE BEGIN FETCH 1 FROM emp_cur INTO x, y; END + -- normalized! +DECLARE +BEGIN +FETCH 1 FROM emp_cur INTO x, y; +END + -- fully parenthesized +DECLARE +BEGIN +FETCH 1 FROM emp_cur INTO x, y; +END + -- literals removed +DECLARE +BEGIN +FETCH 1 FROM _ INTO _, _; +END + -- identifiers removed parse DECLARE @@ -41,6 +89,22 @@ DECLARE BEGIN FETCH 1 FROM emp_cur INTO x, y; END + -- normalized! +DECLARE +BEGIN +FETCH 1 FROM emp_cur INTO x, y; +END + -- fully parenthesized +DECLARE +BEGIN +FETCH 1 FROM emp_cur INTO x, y; +END + -- literals removed +DECLARE +BEGIN +FETCH 1 FROM _ INTO _, _; +END + -- identifiers removed parse DECLARE @@ -52,6 +116,22 @@ DECLARE BEGIN FETCH ABSOLUTE 2 FROM emp_cur INTO x, y; END + -- normalized! +DECLARE +BEGIN +FETCH ABSOLUTE 2 FROM emp_cur INTO x, y; +END + -- fully parenthesized +DECLARE +BEGIN +FETCH ABSOLUTE 2 FROM emp_cur INTO x, y; +END + -- literals removed +DECLARE +BEGIN +FETCH ABSOLUTE 2 FROM _ INTO _, _; +END + -- identifiers removed parse DECLARE @@ -63,30 +143,65 @@ DECLARE BEGIN FETCH 1 FROM emp_cur INTO x; END + -- normalized! +DECLARE +BEGIN +FETCH 1 FROM emp_cur INTO x; +END + -- fully parenthesized +DECLARE +BEGIN +FETCH 1 FROM emp_cur INTO x; +END + -- literals removed +DECLARE +BEGIN +FETCH 1 FROM _ INTO _; +END + -- identifiers removed -parse +error DECLARE BEGIN FETCH emp_cur INTO; END ---- at or near "into": syntax error: missing expression +DETAIL: source SQL: +DECLARE +BEGIN +FETCH emp_cur INTO; + ^ -parse +error DECLARE BEGIN FETCH emp_cur; END ---- at or near "emp_cur": syntax error: invalid syntax for FETCH +DETAIL: source SQL: +DECLARE +BEGIN +FETCH emp_cur; + ^ -parse +error DECLARE BEGIN MOVE NEXT FROM emp_cur INTO x, y; END ---- at or near ";": at or near "x": syntax error +DETAIL: source SQL: +x, y +^ +-- +source SQL: +DECLARE +BEGIN +MOVE NEXT FROM emp_cur INTO x, y; + ^ parse DECLARE @@ -98,6 +213,22 @@ DECLARE BEGIN MOVE 1 FROM emp_cur; END + -- normalized! +DECLARE +BEGIN +MOVE 1 FROM emp_cur; +END + -- fully parenthesized +DECLARE +BEGIN +MOVE 1 FROM emp_cur; +END + -- literals removed +DECLARE +BEGIN +MOVE 1 FROM _; +END + -- identifiers removed parse DECLARE @@ -109,6 +240,22 @@ DECLARE BEGIN MOVE -1 FROM emp_cur; END + -- normalized! +DECLARE +BEGIN +MOVE -1 FROM emp_cur; +END + -- fully parenthesized +DECLARE +BEGIN +MOVE -1 FROM emp_cur; +END + -- literals removed +DECLARE +BEGIN +MOVE -1 FROM _; +END + -- identifiers removed parse DECLARE @@ -120,6 +267,22 @@ DECLARE BEGIN MOVE FIRST FROM emp_cur; END + -- normalized! +DECLARE +BEGIN +MOVE FIRST FROM emp_cur; +END + -- fully parenthesized +DECLARE +BEGIN +MOVE FIRST FROM emp_cur; +END + -- literals removed +DECLARE +BEGIN +MOVE FIRST FROM _; +END + -- identifiers removed parse DECLARE @@ -131,6 +294,22 @@ DECLARE BEGIN MOVE LAST FROM emp_cur; END + -- normalized! +DECLARE +BEGIN +MOVE LAST FROM emp_cur; +END + -- fully parenthesized +DECLARE +BEGIN +MOVE LAST FROM emp_cur; +END + -- literals removed +DECLARE +BEGIN +MOVE LAST FROM _; +END + -- identifiers removed parse DECLARE @@ -142,6 +321,22 @@ DECLARE BEGIN MOVE ABSOLUTE 5 FROM emp_cur; END + -- normalized! +DECLARE +BEGIN +MOVE ABSOLUTE 5 FROM emp_cur; +END + -- fully parenthesized +DECLARE +BEGIN +MOVE ABSOLUTE 5 FROM emp_cur; +END + -- literals removed +DECLARE +BEGIN +MOVE ABSOLUTE 5 FROM _; +END + -- identifiers removed parse DECLARE @@ -153,6 +348,22 @@ DECLARE BEGIN MOVE FIRST FROM emp_cur; END + -- normalized! +DECLARE +BEGIN +MOVE FIRST FROM emp_cur; +END + -- fully parenthesized +DECLARE +BEGIN +MOVE FIRST FROM emp_cur; +END + -- literals removed +DECLARE +BEGIN +MOVE FIRST FROM _; +END + -- identifiers removed parse DECLARE @@ -164,6 +375,22 @@ DECLARE BEGIN MOVE RELATIVE 3 FROM emp_cur; END + -- normalized! +DECLARE +BEGIN +MOVE RELATIVE 3 FROM emp_cur; +END + -- fully parenthesized +DECLARE +BEGIN +MOVE RELATIVE 3 FROM emp_cur; +END + -- literals removed +DECLARE +BEGIN +MOVE RELATIVE 3 FROM _; +END + -- identifiers removed parse DECLARE @@ -175,6 +402,22 @@ DECLARE BEGIN MOVE 3 FROM emp_cur; END + -- normalized! +DECLARE +BEGIN +MOVE 3 FROM emp_cur; +END + -- fully parenthesized +DECLARE +BEGIN +MOVE 3 FROM emp_cur; +END + -- literals removed +DECLARE +BEGIN +MOVE 3 FROM _; +END + -- identifiers removed parse DECLARE @@ -186,6 +429,22 @@ DECLARE BEGIN MOVE -3 FROM emp_cur; END + -- normalized! +DECLARE +BEGIN +MOVE -3 FROM emp_cur; +END + -- fully parenthesized +DECLARE +BEGIN +MOVE -3 FROM emp_cur; +END + -- literals removed +DECLARE +BEGIN +MOVE -3 FROM _; +END + -- identifiers removed parse DECLARE @@ -197,6 +456,22 @@ DECLARE BEGIN MOVE ALL FROM emp_cur; END + -- normalized! +DECLARE +BEGIN +MOVE ALL FROM emp_cur; +END + -- fully parenthesized +DECLARE +BEGIN +MOVE ALL FROM emp_cur; +END + -- literals removed +DECLARE +BEGIN +MOVE ALL FROM _; +END + -- identifiers removed parse DECLARE @@ -208,6 +483,22 @@ DECLARE BEGIN MOVE BACKWARD ALL FROM emp_cur; END + -- normalized! +DECLARE +BEGIN +MOVE BACKWARD ALL FROM emp_cur; +END + -- fully parenthesized +DECLARE +BEGIN +MOVE BACKWARD ALL FROM emp_cur; +END + -- literals removed +DECLARE +BEGIN +MOVE BACKWARD ALL FROM _; +END + -- identifiers removed parse DECLARE @@ -219,6 +510,22 @@ DECLARE BEGIN FETCH 1 FROM emp_cur INTO x; END + -- normalized! +DECLARE +BEGIN +FETCH 1 FROM emp_cur INTO x; +END + -- fully parenthesized +DECLARE +BEGIN +FETCH 1 FROM emp_cur INTO x; +END + -- literals removed +DECLARE +BEGIN +FETCH 1 FROM _ INTO _; +END + -- identifiers removed parse DECLARE @@ -230,6 +537,22 @@ DECLARE BEGIN FETCH -1 FROM emp_cur INTO x; END + -- normalized! +DECLARE +BEGIN +FETCH -1 FROM emp_cur INTO x; +END + -- fully parenthesized +DECLARE +BEGIN +FETCH -1 FROM emp_cur INTO x; +END + -- literals removed +DECLARE +BEGIN +FETCH -1 FROM _ INTO _; +END + -- identifiers removed parse DECLARE @@ -241,6 +564,22 @@ DECLARE BEGIN FETCH FIRST FROM emp_cur INTO x; END + -- normalized! +DECLARE +BEGIN +FETCH FIRST FROM emp_cur INTO x; +END + -- fully parenthesized +DECLARE +BEGIN +FETCH FIRST FROM emp_cur INTO x; +END + -- literals removed +DECLARE +BEGIN +FETCH FIRST FROM _ INTO _; +END + -- identifiers removed parse DECLARE @@ -252,6 +591,22 @@ DECLARE BEGIN FETCH LAST FROM emp_cur INTO x; END + -- normalized! +DECLARE +BEGIN +FETCH LAST FROM emp_cur INTO x; +END + -- fully parenthesized +DECLARE +BEGIN +FETCH LAST FROM emp_cur INTO x; +END + -- literals removed +DECLARE +BEGIN +FETCH LAST FROM _ INTO _; +END + -- identifiers removed parse DECLARE @@ -263,6 +618,22 @@ DECLARE BEGIN FETCH ABSOLUTE 5 FROM emp_cur INTO x; END + -- normalized! +DECLARE +BEGIN +FETCH ABSOLUTE 5 FROM emp_cur INTO x; +END + -- fully parenthesized +DECLARE +BEGIN +FETCH ABSOLUTE 5 FROM emp_cur INTO x; +END + -- literals removed +DECLARE +BEGIN +FETCH ABSOLUTE 5 FROM _ INTO _; +END + -- identifiers removed parse DECLARE @@ -274,6 +645,22 @@ DECLARE BEGIN FETCH FIRST FROM emp_cur INTO x; END + -- normalized! +DECLARE +BEGIN +FETCH FIRST FROM emp_cur INTO x; +END + -- fully parenthesized +DECLARE +BEGIN +FETCH FIRST FROM emp_cur INTO x; +END + -- literals removed +DECLARE +BEGIN +FETCH FIRST FROM _ INTO _; +END + -- identifiers removed parse DECLARE @@ -285,6 +672,22 @@ DECLARE BEGIN FETCH RELATIVE 3 FROM emp_cur INTO x; END + -- normalized! +DECLARE +BEGIN +FETCH RELATIVE 3 FROM emp_cur INTO x; +END + -- fully parenthesized +DECLARE +BEGIN +FETCH RELATIVE 3 FROM emp_cur INTO x; +END + -- literals removed +DECLARE +BEGIN +FETCH RELATIVE 3 FROM _ INTO _; +END + -- identifiers removed parse DECLARE @@ -296,6 +699,22 @@ DECLARE BEGIN FETCH 3 FROM emp_cur INTO x; END + -- normalized! +DECLARE +BEGIN +FETCH 3 FROM emp_cur INTO x; +END + -- fully parenthesized +DECLARE +BEGIN +FETCH 3 FROM emp_cur INTO x; +END + -- literals removed +DECLARE +BEGIN +FETCH 3 FROM _ INTO _; +END + -- identifiers removed parse DECLARE @@ -307,6 +726,22 @@ DECLARE BEGIN FETCH -3 FROM emp_cur INTO x; END + -- normalized! +DECLARE +BEGIN +FETCH -3 FROM emp_cur INTO x; +END + -- fully parenthesized +DECLARE +BEGIN +FETCH -3 FROM emp_cur INTO x; +END + -- literals removed +DECLARE +BEGIN +FETCH -3 FROM _ INTO _; +END + -- identifiers removed parse DECLARE @@ -318,6 +753,22 @@ DECLARE BEGIN FETCH ALL FROM emp_cur INTO x; END + -- normalized! +DECLARE +BEGIN +FETCH ALL FROM emp_cur INTO x; +END + -- fully parenthesized +DECLARE +BEGIN +FETCH ALL FROM emp_cur INTO x; +END + -- literals removed +DECLARE +BEGIN +FETCH ALL FROM _ INTO _; +END + -- identifiers removed parse DECLARE @@ -329,3 +780,19 @@ DECLARE BEGIN FETCH BACKWARD ALL FROM emp_cur INTO x; END + -- normalized! +DECLARE +BEGIN +FETCH BACKWARD ALL FROM emp_cur INTO x; +END + -- fully parenthesized +DECLARE +BEGIN +FETCH BACKWARD ALL FROM emp_cur INTO x; +END + -- literals removed +DECLARE +BEGIN +FETCH BACKWARD ALL FROM _ INTO _; +END + -- identifiers removed diff --git a/pkg/sql/plpgsql/parser/testdata/stmt_for b/pkg/sql/plpgsql/parser/testdata/stmt_for index e49a39ef4993..5dd2a81cad2a 100644 --- a/pkg/sql/plpgsql/parser/testdata/stmt_for +++ b/pkg/sql/plpgsql/parser/testdata/stmt_for @@ -1,4 +1,4 @@ -parse +error DECLARE BEGIN FOR counter IN 1..5 LOOP @@ -6,10 +6,28 @@ FOR counter IN 1..5 LOOP END LOOP; END ---- +---- at or near "counter": syntax error: unimplemented: this syntax +DETAIL: source SQL: +DECLARE +BEGIN +FOR counter IN 1..5 LOOP + ^ +HINT: You have attempted to use a feature that is not yet implemented. + +Please check the public issue tracker to check whether this problem is +already tracked. If you cannot find it there, please report the error +with details by creating a new issue. +If you would rather not post publicly, please contact us directly +using the support form. -parse +We appreciate your feedback. +---- +---- + + +error DECLARE BEGIN <> @@ -18,9 +36,28 @@ FOR counter IN 1..5 LOOP END LOOP for_loop; END ---- +---- at or near "counter": syntax error: unimplemented: this syntax +DETAIL: source SQL: +DECLARE +BEGIN +<> +FOR counter IN 1..5 LOOP + ^ +HINT: You have attempted to use a feature that is not yet implemented. + +Please check the public issue tracker to check whether this problem is +already tracked. If you cannot find it there, please report the error +with details by creating a new issue. + +If you would rather not post publicly, please contact us directly +using the support form. + +We appreciate your feedback. +---- +---- -parse +error DECLARE BEGIN FOR counter IN 1..5 LOOP @@ -28,9 +65,27 @@ FOR counter IN 1..5 LOOP END LOOP; END ---- +---- at or near "counter": syntax error: unimplemented: this syntax +DETAIL: source SQL: +DECLARE +BEGIN +FOR counter IN 1..5 LOOP + ^ +HINT: You have attempted to use a feature that is not yet implemented. + +Please check the public issue tracker to check whether this problem is +already tracked. If you cannot find it there, please report the error +with details by creating a new issue. + +If you would rather not post publicly, please contact us directly +using the support form. + +We appreciate your feedback. +---- +---- -parse +error DECLARE BEGIN FOR yr IN SELECT * FROM generate_series(1,10,1) AS y_(y) @@ -39,4 +94,22 @@ LOOP END LOOP; RETURN; ---- +---- at or near "yr": syntax error: unimplemented: this syntax +DETAIL: source SQL: +DECLARE +BEGIN +FOR yr IN SELECT * FROM generate_series(1,10,1) AS y_(y) + ^ +HINT: You have attempted to use a feature that is not yet implemented. + +Please check the public issue tracker to check whether this problem is +already tracked. If you cannot find it there, please report the error +with details by creating a new issue. + +If you would rather not post publicly, please contact us directly +using the support form. + +We appreciate your feedback. +---- +---- diff --git a/pkg/sql/plpgsql/parser/testdata/stmt_foreach_a b/pkg/sql/plpgsql/parser/testdata/stmt_foreach_a index 9bdbdb3e4899..5c47c7d4f9b7 100644 --- a/pkg/sql/plpgsql/parser/testdata/stmt_foreach_a +++ b/pkg/sql/plpgsql/parser/testdata/stmt_foreach_a @@ -1,4 +1,4 @@ -parse +error DECLARE s int8 := 0; x int; @@ -10,4 +10,24 @@ BEGIN RETURN s; END ---- +---- at or near "x": syntax error: unimplemented: this syntax +DETAIL: source SQL: +DECLARE + s int8 := 0; + x int; +BEGIN + FOREACH x IN ARRAY $1 + ^ +HINT: You have attempted to use a feature that is not yet implemented. + +Please check the public issue tracker to check whether this problem is +already tracked. If you cannot find it there, please report the error +with details by creating a new issue. + +If you would rather not post publicly, please contact us directly +using the support form. + +We appreciate your feedback. +---- +---- diff --git a/pkg/sql/plpgsql/parser/testdata/stmt_getdiag b/pkg/sql/plpgsql/parser/testdata/stmt_getdiag index f9faf6bc0a5d..300e7f3479dc 100644 --- a/pkg/sql/plpgsql/parser/testdata/stmt_getdiag +++ b/pkg/sql/plpgsql/parser/testdata/stmt_getdiag @@ -1,12 +1,9 @@ -parse +feature-count DECLARE BEGIN GET CURRENT DIAGNOSTICS hello := ROW_COUNT; GET STACKED DIAGNOSTICS hello := COLUMN_NAME; END ---- -DECLARE -BEGIN -GET DIAGNOSTICS hello := ROW_COUNT -GET STACKED DIAGNOSTICS hello := COLUMN_NAME -END +stmt_block: 1 +stmt_get_diag: 2 diff --git a/pkg/sql/plpgsql/parser/testdata/stmt_if b/pkg/sql/plpgsql/parser/testdata/stmt_if index 1bbc1382801a..db625b02c01f 100644 --- a/pkg/sql/plpgsql/parser/testdata/stmt_if +++ b/pkg/sql/plpgsql/parser/testdata/stmt_if @@ -14,7 +14,7 @@ END DECLARE BEGIN IF johnnygyro THEN - NULL + NULL; diego := 1 + 2; ELSIF hihotpants THEN diego := 7 + 7; @@ -22,6 +22,43 @@ ELSE diego := 0; END IF; END + -- normalized! +DECLARE +BEGIN +IF (johnnygyro) THEN + NULL; + diego := ((1) + (2)); +ELSIF (hihotpants) THEN + diego := ((7) + (7)); +ELSE + diego := (0); +END IF; +END + -- fully parenthesized +DECLARE +BEGIN +IF johnnygyro THEN + NULL; + diego := _ + _; +ELSIF hihotpants THEN + diego := _ + _; +ELSE + diego := _; +END IF; +END + -- literals removed +DECLARE +BEGIN +IF _ THEN + NULL; + _ := 1 + 2; +ELSIF _ THEN + _ := 7 + 7; +ELSE + _ := 0; +END IF; +END + -- identifiers removed parse @@ -36,6 +73,25 @@ BEGIN IF johnnygyro THEN END IF; END + -- normalized! +DECLARE +BEGIN +IF (johnnygyro) THEN +END IF; +END + -- fully parenthesized +DECLARE +BEGIN +IF johnnygyro THEN +END IF; +END + -- literals removed +DECLARE +BEGIN +IF _ THEN +END IF; +END + -- identifiers removed feature-count DECLARE diff --git a/pkg/sql/plpgsql/parser/testdata/stmt_loop b/pkg/sql/plpgsql/parser/testdata/stmt_loop index f5f6a5c48baf..e7e0ed0063d3 100644 --- a/pkg/sql/plpgsql/parser/testdata/stmt_loop +++ b/pkg/sql/plpgsql/parser/testdata/stmt_loop @@ -16,6 +16,34 @@ EXIT WHEN x = 10; x := x + 1; END LOOP; END + -- normalized! +DECLARE +BEGIN +x := (1); +LOOP +EXIT WHEN ((x) = (10)); +x := ((x) + (1)); +END LOOP; +END + -- fully parenthesized +DECLARE +BEGIN +x := _; +LOOP +EXIT WHEN x = _; +x := x + _; +END LOOP; +END + -- literals removed +DECLARE +BEGIN +_ := 1; +LOOP +EXIT WHEN _ = 10; +_ := _ + 1; +END LOOP; +END + -- identifiers removed parse @@ -32,8 +60,139 @@ END DECLARE BEGIN x := 1; +<> LOOP EXIT WHEN x = 10; x := x + 1; END LOOP mathing; END + -- normalized! +DECLARE +BEGIN +x := (1); +<> +LOOP +EXIT WHEN ((x) = (10)); +x := ((x) + (1)); +END LOOP mathing; +END + -- fully parenthesized +DECLARE +BEGIN +x := _; +<> +LOOP +EXIT WHEN x = _; +x := x + _; +END LOOP mathing; +END + -- literals removed +DECLARE +BEGIN +_ := 1; +<<_>> +LOOP +EXIT WHEN _ = 10; +_ := _ + 1; +END LOOP _; +END + -- identifiers removed + +# The end label can be omitted. +parse +DECLARE +BEGIN +x := 1; +<> +LOOP + EXIT WHEN x = 10; + x := x + 1; +END LOOP; +END +---- +DECLARE +BEGIN +x := 1; +<> +LOOP +EXIT WHEN x = 10; +x := x + 1; +END LOOP mathing; +END + -- normalized! +DECLARE +BEGIN +x := (1); +<> +LOOP +EXIT WHEN ((x) = (10)); +x := ((x) + (1)); +END LOOP mathing; +END + -- fully parenthesized +DECLARE +BEGIN +x := _; +<> +LOOP +EXIT WHEN x = _; +x := x + _; +END LOOP mathing; +END + -- literals removed +DECLARE +BEGIN +_ := 1; +<<_>> +LOOP +EXIT WHEN _ = 10; +_ := _ + 1; +END LOOP _; +END + -- identifiers removed + +# The start and end labels must be the same. +error +DECLARE +BEGIN +x := 1; +<> +LOOP + EXIT WHEN x = 10; + x := x + 1; +END LOOP different_label; +END +---- +at or near ";": syntax error: end label "different_label" differs from block's label "mathing" +DETAIL: source SQL: +DECLARE +BEGIN +x := 1; +<> +LOOP + EXIT WHEN x = 10; + x := x + 1; +END LOOP different_label; + ^ + +# The start label must exist if the end label exists. +error +DECLARE +BEGIN +x := 1; +LOOP + EXIT WHEN x = 10; + x := x + 1; +END LOOP nonempty_label; +END +---- +at or near ";": syntax error: end label "nonempty_label" specified for unlabeled block +DETAIL: source SQL: +DECLARE +BEGIN +x := 1; +LOOP + EXIT WHEN x = 10; + x := x + 1; +END LOOP nonempty_label; + ^ diff --git a/pkg/sql/plpgsql/parser/testdata/stmt_open b/pkg/sql/plpgsql/parser/testdata/stmt_open index d56a7fcffeea..096fc5b6b569 100644 --- a/pkg/sql/plpgsql/parser/testdata/stmt_open +++ b/pkg/sql/plpgsql/parser/testdata/stmt_open @@ -8,6 +8,22 @@ DECLARE BEGIN OPEN curs1; END + -- normalized! +DECLARE +BEGIN +OPEN curs1; +END + -- fully parenthesized +DECLARE +BEGIN +OPEN curs1; +END + -- literals removed +DECLARE +BEGIN +OPEN _; +END + -- identifiers removed parse DECLARE @@ -19,6 +35,22 @@ DECLARE BEGIN OPEN curs1 FOR SELECT * FROM foo WHERE key = mykey; END + -- normalized! +DECLARE +BEGIN +OPEN curs1 FOR SELECT (*) FROM foo WHERE ((key) = (mykey)); +END + -- fully parenthesized +DECLARE +BEGIN +OPEN curs1 FOR SELECT * FROM foo WHERE key = mykey; +END + -- literals removed +DECLARE +BEGIN +OPEN _ FOR SELECT * FROM _ WHERE _ = _; +END + -- identifiers removed parse DECLARE @@ -30,6 +62,22 @@ DECLARE BEGIN OPEN curs1 SCROLL FOR SELECT * FROM foo WHERE key = mykey; END + -- normalized! +DECLARE +BEGIN +OPEN curs1 SCROLL FOR SELECT (*) FROM foo WHERE ((key) = (mykey)); +END + -- fully parenthesized +DECLARE +BEGIN +OPEN curs1 SCROLL FOR SELECT * FROM foo WHERE key = mykey; +END + -- literals removed +DECLARE +BEGIN +OPEN _ SCROLL FOR SELECT * FROM _ WHERE _ = _; +END + -- identifiers removed parse DECLARE @@ -41,19 +89,58 @@ DECLARE BEGIN OPEN curs1 NO SCROLL FOR SELECT * FROM foo WHERE key = mykey; END + -- normalized! +DECLARE +BEGIN +OPEN curs1 NO SCROLL FOR SELECT (*) FROM foo WHERE ((key) = (mykey)); +END + -- fully parenthesized +DECLARE +BEGIN +OPEN curs1 NO SCROLL FOR SELECT * FROM foo WHERE key = mykey; +END + -- literals removed +DECLARE +BEGIN +OPEN _ NO SCROLL FOR SELECT * FROM _ WHERE _ = _; +END + -- identifiers removed -parse +error DECLARE BEGIN OPEN curs2 SCROLL FOR EXECUTE SELECT $1, $2 FROM foo WHERE key = mykey USING hello, jojo; END ---- +---- at or near "execute": syntax error: unimplemented: this syntax +DETAIL: source SQL: +DECLARE +BEGIN +OPEN curs2 SCROLL FOR EXECUTE SELECT $1, $2 FROM foo WHERE key = mykey USING hello, jojo; + ^ +HINT: You have attempted to use a feature that is not yet implemented. -parse +Please check the public issue tracker to check whether this problem is +already tracked. If you cannot find it there, please report the error +with details by creating a new issue. + +If you would rather not post publicly, please contact us directly +using the support form. + +We appreciate your feedback. +---- +---- + +error DECLARE BEGIN OPEN curs1 FOR; END ---- at or near "for": syntax error: missing SQL statement +DETAIL: source SQL: +DECLARE +BEGIN +OPEN curs1 FOR; + ^ diff --git a/pkg/sql/plpgsql/parser/testdata/stmt_perform b/pkg/sql/plpgsql/parser/testdata/stmt_perform index 159e4dc9d27e..f37ae833c6ae 100644 --- a/pkg/sql/plpgsql/parser/testdata/stmt_perform +++ b/pkg/sql/plpgsql/parser/testdata/stmt_perform @@ -1,15 +1,51 @@ -parse +error DECLARE BEGIN PERFORM 1+1; END ---- +---- at or near ";": syntax error: unimplemented: this syntax +DETAIL: source SQL: +DECLARE +BEGIN + PERFORM 1+1; + ^ +HINT: You have attempted to use a feature that is not yet implemented. + +Please check the public issue tracker to check whether this problem is +already tracked. If you cannot find it there, please report the error +with details by creating a new issue. + +If you would rather not post publicly, please contact us directly +using the support form. + +We appreciate your feedback. +---- +---- -parse +error DECLARE BEGIN PERFORM SELECT * FROM generate_series(1,10,1) AS y_(y); END ---- +---- at or near ";": syntax error: unimplemented: this syntax +DETAIL: source SQL: +DECLARE +BEGIN + PERFORM SELECT * FROM generate_series(1,10,1) AS y_(y); + ^ +HINT: You have attempted to use a feature that is not yet implemented. + +Please check the public issue tracker to check whether this problem is +already tracked. If you cannot find it there, please report the error +with details by creating a new issue. + +If you would rather not post publicly, please contact us directly +using the support form. + +We appreciate your feedback. +---- +---- diff --git a/pkg/sql/plpgsql/parser/testdata/stmt_raise b/pkg/sql/plpgsql/parser/testdata/stmt_raise index 02428278b6d7..ef824f0348b6 100644 --- a/pkg/sql/plpgsql/parser/testdata/stmt_raise +++ b/pkg/sql/plpgsql/parser/testdata/stmt_raise @@ -1,10 +1,28 @@ -parse +error DECLARE BEGIN RAISE; END ---- +---- at or near ";": syntax error: unimplemented: this syntax +DETAIL: source SQL: +DECLARE +BEGIN + RAISE; + ^ +HINT: You have attempted to use a feature that is not yet implemented. + +Please check the public issue tracker to check whether this problem is +already tracked. If you cannot find it there, please report the error +with details by creating a new issue. + +If you would rather not post publicly, please contact us directly +using the support form. + +We appreciate your feedback. +---- +---- parse DECLARE @@ -17,6 +35,25 @@ BEGIN RAISE exception USING MESSAGE = "why is this so involved?"; END + -- normalized! +DECLARE +BEGIN +RAISE exception +USING MESSAGE = ("why is this so involved?"); +END + -- fully parenthesized +DECLARE +BEGIN +RAISE exception +USING MESSAGE = "why is this so involved?"; +END + -- literals removed +DECLARE +BEGIN +RAISE exception +USING MESSAGE = _; +END + -- identifiers removed parse DECLARE @@ -29,6 +66,25 @@ BEGIN RAISE log USING HINT = "Insert HINT"; END + -- normalized! +DECLARE +BEGIN +RAISE log +USING HINT = ("Insert HINT"); +END + -- fully parenthesized +DECLARE +BEGIN +RAISE log +USING HINT = "Insert HINT"; +END + -- literals removed +DECLARE +BEGIN +RAISE log +USING HINT = _; +END + -- identifiers removed parse DECLARE @@ -40,6 +96,22 @@ DECLARE BEGIN RAISE log 'Nonexistent ID --> %', user_id; END + -- normalized! +DECLARE +BEGIN +RAISE log 'Nonexistent ID --> %', (user_id); +END + -- fully parenthesized +DECLARE +BEGIN +RAISE log '_', user_id; +END + -- literals removed +DECLARE +BEGIN +RAISE log 'Nonexistent ID --> %', _; +END + -- identifiers removed parse DECLARE @@ -53,6 +125,25 @@ BEGIN RAISE log 'Nonexistent ID --> %', user_id USING HINT = "check...userid?"; END + -- normalized! +DECLARE +BEGIN +RAISE log 'Nonexistent ID --> %', (user_id) +USING HINT = ("check...userid?"); +END + -- fully parenthesized +DECLARE +BEGIN +RAISE log '_', user_id +USING HINT = "check...userid?"; +END + -- literals removed +DECLARE +BEGIN +RAISE log 'Nonexistent ID --> %', _ +USING HINT = _; +END + -- identifiers removed parse DECLARE @@ -64,6 +155,22 @@ DECLARE BEGIN RAISE 'foo %', 'bar'; END + -- normalized! +DECLARE +BEGIN +RAISE 'foo %', ('bar'); +END + -- fully parenthesized +DECLARE +BEGIN +RAISE '_', '_'; +END + -- literals removed +DECLARE +BEGIN +RAISE 'foo %', 'bar'; +END + -- identifiers removed parse DECLARE @@ -77,6 +184,25 @@ i INT8 := 0; BEGIN RAISE 'foo %', i; END + -- normalized! +DECLARE +i INT8 := (0); +BEGIN +RAISE 'foo %', (i); +END + -- fully parenthesized +DECLARE +i INT8 := _; +BEGIN +RAISE '_', i; +END + -- literals removed +DECLARE +i INT8 := 0; +BEGIN +RAISE 'foo %', _; +END + -- identifiers removed parse DECLARE @@ -90,6 +216,25 @@ i INT8 := 0; BEGIN RAISE 'foo %, %, %.', i, i * 2, i * 100; END + -- normalized! +DECLARE +i INT8 := (0); +BEGIN +RAISE 'foo %, %, %.', (i), ((i) * (2)), ((i) * (100)); +END + -- fully parenthesized +DECLARE +i INT8 := _; +BEGIN +RAISE '_', i, i * _, i * _; +END + -- literals removed +DECLARE +i INT8 := 0; +BEGIN +RAISE 'foo %, %, %.', _, _ * 2, _ * 100; +END + -- identifiers removed parse DECLARE @@ -103,6 +248,25 @@ i INT8 := 0; BEGIN RAISE 'foo %', (SELECT count(*) FROM xy WHERE x = i); END + -- normalized! +DECLARE +i INT8 := (0); +BEGIN +RAISE 'foo %', ((SELECT (count((*))) FROM xy WHERE ((x) = (i)))); +END + -- fully parenthesized +DECLARE +i INT8 := _; +BEGIN +RAISE '_', (SELECT count(*) FROM xy WHERE x = i); +END + -- literals removed +DECLARE +i INT8 := 0; +BEGIN +RAISE 'foo %', (SELECT _(*) FROM _ WHERE _ = _); +END + -- identifiers removed parse DECLARE @@ -115,6 +279,25 @@ BEGIN RAISE SQLSTATE '222222' USING HINT = hm; END + -- normalized! +DECLARE +BEGIN +RAISE SQLSTATE '222222' +USING HINT = (hm); +END + -- fully parenthesized +DECLARE +BEGIN +RAISE SQLSTATE '_' +USING HINT = hm; +END + -- literals removed +DECLARE +BEGIN +RAISE SQLSTATE '222222' +USING HINT = _; +END + -- identifiers removed parse DECLARE @@ -126,6 +309,22 @@ DECLARE BEGIN RAISE internal_screaming; END + -- normalized! +DECLARE +BEGIN +RAISE internal_screaming; +END + -- fully parenthesized +DECLARE +BEGIN +RAISE _; +END + -- literals removed +DECLARE +BEGIN +RAISE internal_screaming; +END + -- identifiers removed parse DECLARE @@ -141,3 +340,25 @@ RAISE internal_screaming USING MESSAGE = 'blah blah blah', COLUMN = 'foo'; END + -- normalized! +DECLARE +BEGIN +RAISE internal_screaming +USING MESSAGE = ('blah blah blah'), +COLUMN = ('foo'); +END + -- fully parenthesized +DECLARE +BEGIN +RAISE _ +USING MESSAGE = '_', +COLUMN = '_'; +END + -- literals removed +DECLARE +BEGIN +RAISE internal_screaming +USING MESSAGE = 'blah blah blah', +COLUMN = 'foo'; +END + -- identifiers removed diff --git a/pkg/sql/plpgsql/parser/testdata/stmt_return b/pkg/sql/plpgsql/parser/testdata/stmt_return index 201fcfae4f7b..0da1d5b8c00e 100644 --- a/pkg/sql/plpgsql/parser/testdata/stmt_return +++ b/pkg/sql/plpgsql/parser/testdata/stmt_return @@ -8,6 +8,22 @@ DECLARE BEGIN RETURN 1 + 2; END + -- normalized! +DECLARE +BEGIN +RETURN ((1) + (2)); +END + -- fully parenthesized +DECLARE +BEGIN +RETURN _ + _; +END + -- literals removed +DECLARE +BEGIN +RETURN 1 + 2; +END + -- identifiers removed parse DECLARE @@ -21,6 +37,25 @@ BEGIN x := 1 + 2; RETURN x; END + -- normalized! +DECLARE +BEGIN +x := ((1) + (2)); +RETURN (x); +END + -- fully parenthesized +DECLARE +BEGIN +x := _ + _; +RETURN x; +END + -- literals removed +DECLARE +BEGIN +_ := 1 + 2; +RETURN _; +END + -- identifiers removed parse DECLARE @@ -32,75 +67,177 @@ DECLARE BEGIN RETURN (1, 'string'); END + -- normalized! +DECLARE +BEGIN +RETURN (((1), ('string'))); +END + -- fully parenthesized +DECLARE +BEGIN +RETURN (_, '_'); +END + -- literals removed +DECLARE +BEGIN +RETURN (1, 'string'); +END + -- identifiers removed -parse +error DECLARE BEGIN RETURN QUERY SELECT 1 + 1; END ---- +---- at or near "query": syntax error: unimplemented: this syntax +DETAIL: source SQL: +DECLARE +BEGIN + RETURN QUERY SELECT 1 + 1; + ^ +HINT: You have attempted to use a feature that is not yet implemented. -parse +Please check the public issue tracker to check whether this problem is +already tracked. If you cannot find it there, please report the error +with details by creating a new issue. + +If you would rather not post publicly, please contact us directly +using the support form. + +We appreciate your feedback. +---- +---- + +error DECLARE BEGIN RETURN QUERY EXECUTE a dynamic command; END ---- +---- at or near "query": syntax error: unimplemented: this syntax +DETAIL: source SQL: +DECLARE +BEGIN + RETURN QUERY EXECUTE a dynamic command; + ^ +HINT: You have attempted to use a feature that is not yet implemented. -parse +Please check the public issue tracker to check whether this problem is +already tracked. If you cannot find it there, please report the error +with details by creating a new issue. + +If you would rather not post publicly, please contact us directly +using the support form. + +We appreciate your feedback. +---- +---- + +error DECLARE BEGIN RETURN NEXT 1 + 1; END ---- +---- at or near "next": syntax error: unimplemented: this syntax +DETAIL: source SQL: +DECLARE +BEGIN + RETURN NEXT 1 + 1; + ^ +HINT: You have attempted to use a feature that is not yet implemented. -parse +Please check the public issue tracker to check whether this problem is +already tracked. If you cannot find it there, please report the error +with details by creating a new issue. + +If you would rather not post publicly, please contact us directly +using the support form. + +We appreciate your feedback. +---- +---- + +error DECLARE BEGIN RETURN; END ---- at or near "return": syntax error: missing expression +DETAIL: source SQL: +DECLARE +BEGIN + RETURN; + ^ -parse +error DECLARE BEGIN RETURN (NULL; END ---- at or near "EOF": syntax error: mismatched parentheses +DETAIL: source SQL: +DECLARE +BEGIN + RETURN (NULL; +END + ^ -parse +error DECLARE BEGIN RETURN NULL); END ---- at or near "null": syntax error: mismatched parentheses +DETAIL: source SQL: +DECLARE +BEGIN + RETURN NULL); + ^ -parse +error DECLARE BEGIN RETURN (1, ('string'); END ---- at or near "EOF": syntax error: mismatched parentheses +DETAIL: source SQL: +DECLARE +BEGIN + RETURN (1, ('string'); +END + ^ -parse +error DECLARE BEGIN RETURN 1, 'string'; END ---- at or near "string": syntax error: query returned 2 columns +DETAIL: source SQL: +DECLARE +BEGIN + RETURN 1, 'string'; + ^ -parse +error DECLARE BEGIN RETURN 1, (2, 3, 4, 5); END ---- at or near ")": syntax error: query returned 2 columns +DETAIL: source SQL: +DECLARE +BEGIN + RETURN 1, (2, 3, 4, 5); + ^ diff --git a/pkg/sql/plpgsql/parser/testdata/stmt_while b/pkg/sql/plpgsql/parser/testdata/stmt_while index 691241e32268..de8bb339a6e5 100644 --- a/pkg/sql/plpgsql/parser/testdata/stmt_while +++ b/pkg/sql/plpgsql/parser/testdata/stmt_while @@ -14,6 +14,31 @@ WHILE x > 0 LOOP x := x - 1; END LOOP; END + -- normalized! +DECLARE +BEGIN +x := (10); +WHILE ((x) > (0)) LOOP +x := ((x) - (1)); +END LOOP; +END + -- fully parenthesized +DECLARE +BEGIN +x := _; +WHILE x > _ LOOP +x := x - _; +END LOOP; +END + -- literals removed +DECLARE +BEGIN +_ := 10; +WHILE _ > 0 LOOP +_ := _ - 1; +END LOOP; +END + -- identifiers removed parse DECLARE @@ -33,6 +58,34 @@ x := x - 1; x := x - 2; END LOOP; END + -- normalized! +DECLARE +BEGIN +x := (10); +WHILE ((((x) > (0))) AND (((x) < (100)))) LOOP +x := ((x) - (1)); +x := ((x) - (2)); +END LOOP; +END + -- fully parenthesized +DECLARE +BEGIN +x := _; +WHILE (x > _) AND (x < _) LOOP +x := x - _; +x := x - _; +END LOOP; +END + -- literals removed +DECLARE +BEGIN +_ := 10; +WHILE (_ > 0) AND (_ < 100) LOOP +_ := _ - 1; +_ := _ - 2; +END LOOP; +END + -- identifiers removed parse DECLARE @@ -47,7 +100,36 @@ END DECLARE BEGIN x := 10; +<> WHILE x > 0 LOOP x := x - 1; END LOOP labeled; END + -- normalized! +DECLARE +BEGIN +x := (10); +<> +WHILE ((x) > (0)) LOOP +x := ((x) - (1)); +END LOOP labeled; +END + -- fully parenthesized +DECLARE +BEGIN +x := _; +<> +WHILE x > _ LOOP +x := x - _; +END LOOP labeled; +END + -- literals removed +DECLARE +BEGIN +_ := 10; +<<_>> +WHILE _ > 0 LOOP +_ := _ - 1; +END LOOP _; +END + -- identifiers removed diff --git a/pkg/sql/sem/plpgsqltree/exception.go b/pkg/sql/sem/plpgsqltree/exception.go index 9dbc83df8a8f..2317d4847d0b 100644 --- a/pkg/sql/sem/plpgsqltree/exception.go +++ b/pkg/sql/sem/plpgsqltree/exception.go @@ -10,11 +10,7 @@ package plpgsqltree -import ( - "fmt" - - "github.com/cockroachdb/cockroach/pkg/sql/sem/tree" -) +import "github.com/cockroachdb/cockroach/pkg/sql/sem/tree" type Exception struct { StatementImpl @@ -36,14 +32,15 @@ func (s *Exception) Format(ctx *tree.FmtCtx) { ctx.WriteString(" OR ") } if cond.SqlErrState != "" { - ctx.WriteString(fmt.Sprintf("SQLSTATE '%s'", cond.SqlErrState)) + ctx.WriteString("SQLSTATE ") + formatStringQuotes(ctx, cond.SqlErrState) } else { - ctx.WriteString(cond.SqlErrName) + formatString(ctx, cond.SqlErrName) } } ctx.WriteString(" THEN\n") for _, stmt := range s.Action { - stmt.Format(ctx) + ctx.FormatNode(stmt) } } diff --git a/pkg/sql/sem/plpgsqltree/statements.go b/pkg/sql/sem/plpgsqltree/statements.go index 2f4ad0cff442..05afc1624db8 100644 --- a/pkg/sql/sem/plpgsqltree/statements.go +++ b/pkg/sql/sem/plpgsqltree/statements.go @@ -77,19 +77,19 @@ func (s *Block) Format(ctx *tree.FmtCtx) { if s.Decls != nil { ctx.WriteString("DECLARE\n") for _, dec := range s.Decls { - dec.Format(ctx) + ctx.FormatNode(dec) } } // TODO(drewk): Make sure the child statement is pretty printed correctly // with indents. ctx.WriteString("BEGIN\n") for _, childStmt := range s.Body { - childStmt.Format(ctx) + ctx.FormatNode(childStmt) } if s.Exceptions != nil { ctx.WriteString("EXCEPTION\n") for _, e := range s.Exceptions { - e.Format(ctx) + ctx.FormatNode(&e) } } ctx.WriteString("END\n") @@ -155,16 +155,18 @@ func (s *Declaration) Format(ctx *tree.FmtCtx) { if s.Constant { ctx.WriteString(" CONSTANT") } - ctx.WriteString(fmt.Sprintf(" %s", s.Typ.SQLString())) + ctx.WriteString(" ") + ctx.FormatTypeReference(s.Typ) if s.Collate != "" { - ctx.WriteString(fmt.Sprintf(" %s", s.Collate)) + ctx.WriteString(" COLLATE ") + ctx.FormatNameP(&s.Collate) } if s.NotNull { ctx.WriteString(" NOT NULL") } if s.Expr != nil { ctx.WriteString(" := ") - s.Expr.Format(ctx) + ctx.FormatNode(s.Expr) } ctx.WriteString(";\n") } @@ -199,7 +201,7 @@ func (s *CursorDeclaration) Format(ctx *tree.FmtCtx) { ctx.WriteString(" NO SCROLL") } ctx.WriteString(" CURSOR FOR ") - s.Query.Format(ctx) + ctx.FormatNode(s.Query) ctx.WriteString(";\n") } @@ -229,7 +231,10 @@ func (s *Assignment) PlpgSQLStatementTag() string { } func (s *Assignment) Format(ctx *tree.FmtCtx) { - ctx.WriteString(fmt.Sprintf("%s := %s;\n", s.Var, s.Value)) + ctx.FormatNode(&s.Var) + ctx.WriteString(" := ") + ctx.FormatNode(s.Value) + ctx.WriteString(";\n") } func (s *Assignment) WalkStmt(visitor StatementVisitor) (newStmt Statement, changed bool) { @@ -260,22 +265,22 @@ func (s *If) CopyNode() *If { func (s *If) Format(ctx *tree.FmtCtx) { ctx.WriteString("IF ") - s.Condition.Format(ctx) + ctx.FormatNode(s.Condition) ctx.WriteString(" THEN\n") for _, stmt := range s.ThenBody { // TODO(drewk): Pretty Print with spaces, not tabs. ctx.WriteString("\t") - stmt.Format(ctx) + ctx.FormatNode(stmt) } for _, elsifStmt := range s.ElseIfList { - elsifStmt.Format(ctx) + ctx.FormatNode(&elsifStmt) } for i, elseStmt := range s.ElseBody { if i == 0 { ctx.WriteString("ELSE\n") } ctx.WriteString("\t") - elseStmt.Format(ctx) + ctx.FormatNode(elseStmt) } ctx.WriteString("END IF;\n") } @@ -337,11 +342,11 @@ func (s *ElseIf) CopyNode() *ElseIf { func (s *ElseIf) Format(ctx *tree.FmtCtx) { ctx.WriteString("ELSIF ") - s.Condition.Format(ctx) + ctx.FormatNode(s.Condition) ctx.WriteString(" THEN\n") for _, stmt := range s.Stmts { ctx.WriteString("\t") - stmt.Format(ctx) + ctx.FormatNode(stmt) } } @@ -397,13 +402,13 @@ func (s *Case) Format(ctx *tree.FmtCtx) { } ctx.WriteString("\n") for _, when := range s.CaseWhenList { - when.Format(ctx) + ctx.FormatNode(when) } if s.HaveElse { ctx.WriteString("ELSE\n") for _, stmt := range s.ElseStmts { ctx.WriteString(" ") - stmt.Format(ctx) + ctx.FormatNode(stmt) } } ctx.WriteString("END CASE\n") @@ -459,7 +464,7 @@ func (s *CaseWhen) Format(ctx *tree.FmtCtx) { ctx.WriteString(fmt.Sprintf("WHEN %s THEN\n", s.Expr)) for i, stmt := range s.Stmts { ctx.WriteString(" ") - stmt.Format(ctx) + ctx.FormatNode(stmt) if i != len(s.Stmts)-1 { ctx.WriteString("\n") } @@ -504,13 +509,19 @@ func (s *Loop) PlpgSQLStatementTag() string { } func (s *Loop) Format(ctx *tree.FmtCtx) { + if s.Label != "" { + ctx.WriteString("<<") + ctx.FormatNameP(&s.Label) + ctx.WriteString(">>\n") + } ctx.WriteString("LOOP\n") for _, stmt := range s.Body { - stmt.Format(ctx) + ctx.FormatNode(stmt) } ctx.WriteString("END LOOP") if s.Label != "" { - ctx.WriteString(fmt.Sprintf(" %s", s.Label)) + ctx.WriteString(" ") + ctx.FormatNameP(&s.Label) } ctx.WriteString(";\n") } @@ -545,15 +556,21 @@ func (s *While) CopyNode() *While { } func (s *While) Format(ctx *tree.FmtCtx) { + if s.Label != "" { + ctx.WriteString("<<") + ctx.FormatNameP(&s.Label) + ctx.WriteString(">>\n") + } ctx.WriteString("WHILE ") - s.Condition.Format(ctx) + ctx.FormatNode(s.Condition) ctx.WriteString(" LOOP\n") for _, stmt := range s.Body { - stmt.Format(ctx) + ctx.FormatNode(stmt) } ctx.WriteString("END LOOP") if s.Label != "" { - ctx.WriteString(fmt.Sprintf(" %s", s.Label)) + ctx.WriteString(" ") + ctx.FormatNameP(&s.Label) } ctx.WriteString(";\n") } @@ -704,11 +721,12 @@ func (s *Exit) CopyNode() *Exit { func (s *Exit) Format(ctx *tree.FmtCtx) { ctx.WriteString("EXIT") if s.Label != "" { - ctx.WriteString(fmt.Sprintf(" %s", s.Label)) + ctx.WriteString(" ") + ctx.FormatNameP(&s.Label) } if s.Condition != nil { ctx.WriteString(" WHEN ") - s.Condition.Format(ctx) + ctx.FormatNode(s.Condition) } ctx.WriteString(";\n") @@ -738,11 +756,12 @@ func (s *Continue) CopyNode() *Continue { func (s *Continue) Format(ctx *tree.FmtCtx) { ctx.WriteString("CONTINUE") if s.Label != "" { - ctx.WriteString(fmt.Sprintf(" %s", s.Label)) + ctx.WriteString(" ") + ctx.FormatNameP(&s.Label) } if s.Condition != nil { ctx.WriteString(" WHEN ") - s.Condition.Format(ctx) + ctx.FormatNode(s.Condition) } ctx.WriteString(";\n") } @@ -771,9 +790,9 @@ func (s *Return) CopyNode() *Return { func (s *Return) Format(ctx *tree.FmtCtx) { ctx.WriteString("RETURN ") if s.Expr == nil { - s.RetVar.Format(ctx) + ctx.FormatNode(&s.RetVar) } else { - s.Expr.Format(ctx) + ctx.FormatNode(s.Expr) } ctx.WriteString(";\n") } @@ -847,16 +866,19 @@ func (s *Raise) Format(ctx *tree.FmtCtx) { ctx.WriteString(s.LogLevel) } if s.Code != "" { - ctx.WriteString(fmt.Sprintf(" SQLSTATE '%s'", s.Code)) + ctx.WriteString(" SQLSTATE ") + formatStringQuotes(ctx, s.Code) } if s.CodeName != "" { - ctx.WriteString(fmt.Sprintf(" %s", s.CodeName)) + ctx.WriteString(" ") + formatString(ctx, s.CodeName) } if s.Message != "" { - ctx.WriteString(fmt.Sprintf(" '%s'", s.Message)) + ctx.WriteString(" ") + formatStringQuotes(ctx, s.Message) for i := range s.Params { ctx.WriteString(", ") - s.Params[i].Format(ctx) + ctx.FormatNode(s.Params[i]) } } for i := range s.Options { @@ -865,7 +887,7 @@ func (s *Raise) Format(ctx *tree.FmtCtx) { } else { ctx.WriteString(",\n") } - s.Options[i].Format(ctx) + ctx.FormatNode(&s.Options[i]) } ctx.WriteString(";\n") } @@ -877,7 +899,7 @@ type RaiseOption struct { func (s *RaiseOption) Format(ctx *tree.FmtCtx) { ctx.WriteString(fmt.Sprintf("%s = ", strings.ToUpper(s.OptType))) - s.Expr.Format(ctx) + ctx.FormatNode(s.Expr) } func (s *Raise) PlpgSQLStatementTag() string { @@ -930,7 +952,7 @@ func (s *Execute) CopyNode() *Execute { } func (s *Execute) Format(ctx *tree.FmtCtx) { - s.SqlStmt.Format(ctx) + ctx.FormatNode(s.SqlStmt) if s.Target != nil { ctx.WriteString(" INTO ") if s.Strict { @@ -940,7 +962,7 @@ func (s *Execute) Format(ctx *tree.FmtCtx) { if i > 0 { ctx.WriteString(", ") } - s.Target[i].Format(ctx) + ctx.FormatNode(&s.Target[i]) } } ctx.WriteString(";\n") @@ -1058,7 +1080,7 @@ func (s *GetDiagnostics) Format(ctx *tree.FmtCtx) { ctx.WriteString("GET DIAGNOSTICS ") } for idx, i := range s.DiagItems { - i.Format(ctx) + ctx.FormatNode(i) if idx != len(s.DiagItems)-1 { ctx.WriteString(" ") } @@ -1103,7 +1125,7 @@ func (s *Open) CopyNode() *Open { func (s *Open) Format(ctx *tree.FmtCtx) { ctx.WriteString("OPEN ") - s.CurVar.Format(ctx) + ctx.FormatNode(&s.CurVar) switch s.Scroll { case tree.Scroll: ctx.WriteString(" SCROLL") @@ -1112,7 +1134,7 @@ func (s *Open) Format(ctx *tree.FmtCtx) { } if s.Query != nil { ctx.WriteString(" FOR ") - s.Query.Format(ctx) + ctx.FormatNode(s.Query) } ctx.WriteString(";\n") } @@ -1150,14 +1172,14 @@ func (s *Fetch) Format(ctx *tree.FmtCtx) { ctx.WriteString(" ") } ctx.WriteString("FROM ") - s.Cursor.Name.Format(ctx) + ctx.FormatName(string(s.Cursor.Name)) if s.Target != nil { ctx.WriteString(" INTO ") for i := range s.Target { if i > 0 { ctx.WriteString(", ") } - s.Target[i].Format(ctx) + ctx.FormatNode(&s.Target[i]) } } ctx.WriteString(";\n") @@ -1183,7 +1205,7 @@ type Close struct { func (s *Close) Format(ctx *tree.FmtCtx) { ctx.WriteString("CLOSE ") - s.CurVar.Format(ctx) + ctx.FormatNode(&s.CurVar) ctx.WriteString(";\n") } @@ -1238,7 +1260,7 @@ type Null struct { } func (s *Null) Format(ctx *tree.FmtCtx) { - ctx.WriteString("NULL\n") + ctx.WriteString("NULL;\n") } func (s *Null) PlpgSQLStatementTag() string { @@ -1249,3 +1271,21 @@ func (s *Null) WalkStmt(visitor StatementVisitor) (newStmt Statement, changed bo newStmt, changed = visitor.Visit(s) return newStmt, changed } + +// formatString is a helper function that prints "_" if FmtHideConstants is set, +// and otherwise prints the given string. +func formatString(ctx *tree.FmtCtx, str string) { + if ctx.HasFlags(tree.FmtHideConstants) { + ctx.WriteString("_") + } else { + ctx.WriteString(str) + } +} + +// formatStringQuotes is similar to formatString, but surrounds the output with +// single quotes. +func formatStringQuotes(ctx *tree.FmtCtx, str string) { + ctx.WriteString("'") + formatString(ctx, str) + ctx.WriteString("'") +} diff --git a/pkg/sql/sem/plpgsqltree/utils/plpg_visitor.go b/pkg/sql/sem/plpgsqltree/utils/plpg_visitor.go index 437419dd8611..43d99b0eb3a6 100644 --- a/pkg/sql/sem/plpgsqltree/utils/plpg_visitor.go +++ b/pkg/sql/sem/plpgsqltree/utils/plpg_visitor.go @@ -128,6 +128,24 @@ type SQLStmtVisitor struct { var _ plpgsqltree.StatementVisitor = &SQLStmtVisitor{} +// simpleStmtVisit wraps tree.SimpleStmtVisit and handles the case when the +// statement is nil for convenience. +func simpleStmtVisit(statement tree.Statement, fn tree.SimpleVisitFn) (tree.Statement, error) { + if statement == nil { + return nil, nil + } + return tree.SimpleStmtVisit(statement, fn) +} + +// simpleVisit wraps tree.SimpleVisit and handles the case when the expression +// is nil for convenience. +func simpleVisit(expr tree.Expr, fn tree.SimpleVisitFn) (tree.Expr, error) { + if expr == nil { + return nil, nil + } + return tree.SimpleVisit(expr, fn) +} + func (v *SQLStmtVisitor) Visit( stmt plpgsqltree.Statement, ) (newStmt plpgsqltree.Statement, changed bool) { @@ -139,7 +157,7 @@ func (v *SQLStmtVisitor) Visit( var e tree.Expr switch t := stmt.(type) { case *plpgsqltree.CursorDeclaration: - s, v.Err = tree.SimpleStmtVisit(t.Query, v.Fn) + s, v.Err = simpleStmtVisit(t.Query, v.Fn) if v.Err != nil { return stmt, false } @@ -150,7 +168,7 @@ func (v *SQLStmtVisitor) Visit( newStmt = cpy } case *plpgsqltree.Execute: - s, v.Err = tree.SimpleStmtVisit(t.SqlStmt, v.Fn) + s, v.Err = simpleStmtVisit(t.SqlStmt, v.Fn) if v.Err != nil { return stmt, false } @@ -161,7 +179,7 @@ func (v *SQLStmtVisitor) Visit( newStmt = cpy } case *plpgsqltree.Open: - s, v.Err = tree.SimpleStmtVisit(t.Query, v.Fn) + s, v.Err = simpleStmtVisit(t.Query, v.Fn) if v.Err != nil { return stmt, false } @@ -172,7 +190,7 @@ func (v *SQLStmtVisitor) Visit( newStmt = cpy } case *plpgsqltree.Declaration: - e, v.Err = tree.SimpleVisit(t.Expr, v.Fn) + e, v.Err = simpleVisit(t.Expr, v.Fn) if v.Err != nil { return stmt, false } @@ -184,7 +202,7 @@ func (v *SQLStmtVisitor) Visit( } case *plpgsqltree.Assignment: - e, v.Err = tree.SimpleVisit(t.Value, v.Fn) + e, v.Err = simpleVisit(t.Value, v.Fn) if v.Err != nil { return stmt, false } @@ -195,7 +213,7 @@ func (v *SQLStmtVisitor) Visit( newStmt = cpy } case *plpgsqltree.If: - e, v.Err = tree.SimpleVisit(t.Condition, v.Fn) + e, v.Err = simpleVisit(t.Condition, v.Fn) if v.Err != nil { return stmt, false } @@ -206,7 +224,7 @@ func (v *SQLStmtVisitor) Visit( newStmt = cpy } case *plpgsqltree.ElseIf: - e, v.Err = tree.SimpleVisit(t.Condition, v.Fn) + e, v.Err = simpleVisit(t.Condition, v.Fn) if v.Err != nil { return stmt, false } @@ -217,7 +235,7 @@ func (v *SQLStmtVisitor) Visit( newStmt = cpy } case *plpgsqltree.While: - e, v.Err = tree.SimpleVisit(t.Condition, v.Fn) + e, v.Err = simpleVisit(t.Condition, v.Fn) if v.Err != nil { return stmt, false } @@ -227,7 +245,7 @@ func (v *SQLStmtVisitor) Visit( cpy.Condition = e newStmt = cpy } - e, v.Err = tree.SimpleVisit(t.Condition, v.Fn) + e, v.Err = simpleVisit(t.Condition, v.Fn) if v.Err != nil { return stmt, false } @@ -238,7 +256,7 @@ func (v *SQLStmtVisitor) Visit( newStmt = cpy } case *plpgsqltree.Exit: - e, v.Err = tree.SimpleVisit(t.Condition, v.Fn) + e, v.Err = simpleVisit(t.Condition, v.Fn) if v.Err != nil { return stmt, false } @@ -249,7 +267,7 @@ func (v *SQLStmtVisitor) Visit( newStmt = cpy } case *plpgsqltree.Continue: - e, v.Err = tree.SimpleVisit(t.Condition, v.Fn) + e, v.Err = simpleVisit(t.Condition, v.Fn) if v.Err != nil { return stmt, false } @@ -260,7 +278,7 @@ func (v *SQLStmtVisitor) Visit( newStmt = cpy } case *plpgsqltree.Return: - e, v.Err = tree.SimpleVisit(t.Expr, v.Fn) + e, v.Err = simpleVisit(t.Expr, v.Fn) if v.Err != nil { return stmt, false } @@ -272,7 +290,7 @@ func (v *SQLStmtVisitor) Visit( } case *plpgsqltree.Raise: for i, p := range t.Params { - e, v.Err = tree.SimpleVisit(p, v.Fn) + e, v.Err = simpleVisit(p, v.Fn) if v.Err != nil { return stmt, false } @@ -286,7 +304,7 @@ func (v *SQLStmtVisitor) Visit( } } for i, p := range t.Options { - e, v.Err = tree.SimpleVisit(p.Expr, v.Fn) + e, v.Err = simpleVisit(p.Expr, v.Fn) if v.Err != nil { return stmt, false } @@ -300,7 +318,7 @@ func (v *SQLStmtVisitor) Visit( } } case *plpgsqltree.Assert: - e, v.Err = tree.SimpleVisit(t.Condition, v.Fn) + e, v.Err = simpleVisit(t.Condition, v.Fn) if v.Err != nil { return stmt, false } @@ -313,7 +331,7 @@ func (v *SQLStmtVisitor) Visit( case *plpgsqltree.DynamicExecute: for i, p := range t.Params { - e, v.Err = tree.SimpleVisit(p, v.Fn) + e, v.Err = simpleVisit(p, v.Fn) if v.Err != nil { return stmt, false } @@ -327,7 +345,7 @@ func (v *SQLStmtVisitor) Visit( } } case *plpgsqltree.Call: - e, v.Err = tree.SimpleVisit(t.Expr, v.Fn) + e, v.Err = simpleVisit(t.Expr, v.Fn) if v.Err != nil { return stmt, false } diff --git a/pkg/testutils/sqlutils/BUILD.bazel b/pkg/testutils/sqlutils/BUILD.bazel index 7f70d6620224..d2efb1aa7655 100644 --- a/pkg/testutils/sqlutils/BUILD.bazel +++ b/pkg/testutils/sqlutils/BUILD.bazel @@ -4,6 +4,7 @@ go_library( name = "sqlutils", srcs = [ "inject.go", + "parse.go", "pg_url.go", "pretty.go", "rows.go", @@ -23,7 +24,9 @@ go_library( "//pkg/sql/catalog/descpb", "//pkg/sql/lexbase", "//pkg/sql/parser", + "//pkg/sql/parser/statements", "//pkg/sql/pgwire/pgerror", + "//pkg/sql/plpgsql/parser:plpgparser", "//pkg/sql/protoreflect", "//pkg/sql/sem/catconstants", "//pkg/sql/sem/tree", diff --git a/pkg/testutils/sqlutils/parse.go b/pkg/testutils/sqlutils/parse.go new file mode 100644 index 000000000000..1da8cd92557c --- /dev/null +++ b/pkg/testutils/sqlutils/parse.go @@ -0,0 +1,127 @@ +// Copyright 2023 The Cockroach Authors. +// +// Use of this software is governed by the Business Source License +// included in the file licenses/BSL.txt. +// +// As of the Change Date specified in that file, in accordance with +// the Business Source License, use of this software will be governed +// by the Apache License, Version 2.0, included in the file +// licenses/APL.txt. + +package sqlutils + +import ( + "bytes" + "fmt" + "regexp" + "strings" + "testing" + + "github.com/cockroachdb/cockroach/pkg/sql/parser" + "github.com/cockroachdb/cockroach/pkg/sql/parser/statements" + "github.com/cockroachdb/cockroach/pkg/sql/pgwire/pgerror" + plpgsqlparser "github.com/cockroachdb/cockroach/pkg/sql/plpgsql/parser" + "github.com/cockroachdb/cockroach/pkg/sql/sem/tree" +) + +func parse(t *testing.T, input string, plpgsql bool) (statements.ParsedStmts, error) { + t.Helper() + if plpgsql { + return plpgsqlparser.Parse(input) + } + return parser.Parse(input) +} + +func parseOne(t *testing.T, input string, plpgsql bool) (tree.NodeFormatter, error) { + t.Helper() + if plpgsql { + stmt, err := plpgsqlparser.Parse(input) + if err != nil { + return nil, err + } + return stmt.AST, err + } + stmt, err := parser.ParseOne(input) + if err != nil { + return nil, err + } + return stmt.AST, err +} + +// VerifyParseFormat is used in the SQL and PL/pgSQL datadriven parser tests to +// check that a successfully parsed expression round trips and correctly handles +// formatting flags. +func VerifyParseFormat(t *testing.T, input string, plpgsql bool) string { + t.Helper() + + // Check parse. + stmts, err := parse(t, input, plpgsql) + if err != nil { + t.Fatalf("unexpected parse error: %v", err) + } + + // Check pretty-print roundtrip. + if plpgsql { + plStmt := stmts.(statements.PLpgStatement).AST + verifyStatementPrettyRoundTrip(t, input, plStmt, true /* plpgsql */) + } else { + VerifyStatementPrettyRoundtrip(t, input) + } + + ref := stmts.StringWithFlags(tree.FmtSimple) + note := "" + if ref != input { + note = " -- normalized!" + } + + // Check roundtrip and formatting with flags. + var buf bytes.Buffer + fmt.Fprintf(&buf, "%s%s\n", ref, note) + fmt.Fprintln(&buf, stmts.StringWithFlags(tree.FmtAlwaysGroupExprs), "-- fully parenthesized") + constantsHidden := stmts.StringWithFlags(tree.FmtHideConstants) + fmt.Fprintln(&buf, constantsHidden, "-- literals removed") + + // As of this writing, the SQL statement stats proceed as follows: + // first the literals are removed from statement to form a stat key, + // then the stat key is re-parsed, to undergo the anonymization stage. + // We also want to check the re-parsing is fine. + reparsedStmts, err := parse(t, constantsHidden, plpgsql) + if err != nil { + t.Fatalf("unexpected error when reparsing without literals: %+v", err) + } else { + reparsedStmtsS := reparsedStmts.String() + if reparsedStmtsS != constantsHidden { + t.Fatalf( + "mismatched AST when reparsing without literals:\noriginal: %s\nexpected: %s\nactual: %s", + input, constantsHidden, reparsedStmtsS, + ) + } + } + + fmt.Fprintln(&buf, stmts.StringWithFlags(tree.FmtAnonymize), "-- identifiers removed") + if strings.Contains(ref, tree.PasswordSubstitution) { + fmt.Fprintln(&buf, stmts.StringWithFlags(tree.FmtShowPasswords), "-- passwords exposed") + } + + return buf.String() +} + +var issueLinkRE = regexp.MustCompile("https://go.crdb.dev/issue-v/([0-9]+)/.*") + +// VerifyParseError is used in the SQL and PL/pgSQL datadriven parser tests to +// check that an unsuccessfully parsed expression returns an expected error. +func VerifyParseError(err error) string { + if err == nil { + return "" + } + pgerr := pgerror.Flatten(err) + msg := pgerr.Message + if pgerr.Detail != "" { + msg += "\nDETAIL: " + pgerr.Detail + } + if pgerr.Hint != "" { + msg += "\nHINT: " + pgerr.Hint + } + msg = issueLinkRE.ReplaceAllString(msg, "https://go.crdb.dev/issue-v/$1/") + return msg +} diff --git a/pkg/testutils/sqlutils/pretty.go b/pkg/testutils/sqlutils/pretty.go index c6e075c795f5..3c4e7b716a64 100644 --- a/pkg/testutils/sqlutils/pretty.go +++ b/pkg/testutils/sqlutils/pretty.go @@ -26,73 +26,81 @@ func VerifyStatementPrettyRoundtrip(t *testing.T, sql string) { if err != nil { t.Fatalf("%s: %s", err, sql) } + for i := range stmts { + origStmt := stmts[i].AST + verifyStatementPrettyRoundTrip(t, sql, origStmt, false /* plpgsql */) + } +} + +// verifyStatementPrettyRoundTrip verifies that a SQL or PL/pgSQL statement +// correctly round trips through the pretty printer. +func verifyStatementPrettyRoundTrip( + t *testing.T, sql string, origStmt tree.NodeFormatter, plpgsql bool, +) { + t.Helper() + // Dataflow of the statement through these checks: + // + // sql (from test file) + // | + // (parser.Parse) + // v + // origStmt + // / \ + // (cfg.Pretty) (AsStringWithFlags,FmtParsable) + // v | + // prettyStmt | + // | | + // (parser.ParseOne) | + // v | + // parsedPretty | + // | | + // (AsStringWithFlags,FmtSimple) | + // v v + // prettyFormatted origFormatted + // + // == Check 1: prettyFormatted == origFormatted + // If false: + // + // | | + // | (parser.ParseOne) + // | v + // | reparsedStmt + // | | + // | (AsStringWithFlags,FmtParsable) + // v v + // prettyFormatted origFormatted + // + // == Check 2: prettyFormatted == origFormatted + // cfg := tree.DefaultPrettyCfg() // Be careful to not simplify otherwise the tests won't round trip. cfg.Simplify = false - for i := range stmts { - // Dataflow of the statement through these checks: - // - // sql (from test file) - // | - // (parser.Parse) - // v - // origStmt - // / \ - // (cfg.Pretty) (AsStringWithFlags,FmtParsable) - // v | - // prettyStmt | - // | | - // (parser.ParseOne) | - // v | - // parsedPretty | - // | | - // (AsStringWithFlags,FmtSimple) | - // v v - // prettyFormatted origFormatted - // - // == Check 1: prettyFormatted == origFormatted - // If false: - // - // | | - // | (parser.ParseOne) - // | v - // | reparsedStmt - // | | - // | (AsStringWithFlags,FmtParsable) - // v v - // prettyFormatted origFormatted - // - // == Check 2: prettyFormatted == origFormatted - // - origStmt := stmts[i].AST - // Be careful to not simplify otherwise the tests won't round trip. - prettyStmt, err := cfg.Pretty(origStmt) - if err != nil { - t.Fatalf("%s: %s", err, prettyStmt) - } - parsedPretty, err := parser.ParseOne(prettyStmt) + prettyStmt, err := cfg.Pretty(origStmt) + if err != nil { + t.Fatalf("%s: %s", err, prettyStmt) + } + parsedPretty, err := parseOne(t, prettyStmt, plpgsql) + if err != nil { + t.Fatalf("%s: %s", err, prettyStmt) + } + prettyFormatted := tree.AsStringWithFlags(parsedPretty, tree.FmtSimple) + origFormatted := tree.AsStringWithFlags(origStmt, tree.FmtParsable) + if prettyFormatted != origFormatted { + // Type annotations and unicode strings don't round trip well. Sometimes we + // need to reparse the original formatted output and format that for these + // to match. + reparsedStmt, err := parseOne(t, origFormatted, plpgsql) if err != nil { - t.Fatalf("%s: %s", err, prettyStmt) + t.Fatal(err) } - prettyFormatted := tree.AsStringWithFlags(parsedPretty.AST, tree.FmtSimple) - origFormatted := tree.AsStringWithFlags(origStmt, tree.FmtParsable) + origFormatted = tree.AsStringWithFlags(reparsedStmt, tree.FmtParsable) if prettyFormatted != origFormatted { - // Type annotations and unicode strings don't round trip well. Sometimes we - // need to reparse the original formatted output and format that for these - // to match. - reparsedStmt, err := parser.ParseOne(origFormatted) - if err != nil { - t.Fatal(err) - } - origFormatted = tree.AsStringWithFlags(reparsedStmt.AST, tree.FmtParsable) - if prettyFormatted != origFormatted { - t.Fatalf("orig formatted != pretty formatted\norig SQL: %q\norig formatted: %q\npretty printed: %s\npretty formatted: %q", - sql, - origFormatted, - prettyStmt, - prettyFormatted, - ) - } + t.Fatalf("orig formatted != pretty formatted\norig SQL: %q\norig formatted: %q\npretty printed: %s\npretty formatted: %q", + sql, + origFormatted, + prettyStmt, + prettyFormatted, + ) } } }