Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implemented a no-op node for statements that can be safely ignored #1103

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

zachmu
Copy link
Member

@zachmu zachmu commented Jan 7, 2025

Used for OWNER ddl statements for now, and will extend to VACUUM next.

@zachmu zachmu requested a review from Hydrocharged January 7, 2025 01:59
Copy link
Contributor

github-actions bot commented Jan 7, 2025

Main PR
covering_index_scan_postgres 369.62/s 367.70/s -0.6%
index_join_postgres 151.74/s 152.60/s +0.5%
index_join_scan_postgres 182.13/s 181.95/s -0.1%
index_scan_postgres 12.40/s 12.50/s +0.8%
oltp_point_select 2748.81/s 2760.16/s +0.4%
oltp_read_only 1857.56/s 1856.61/s -0.1%
select_random_points 111.59/s 111.94/s +0.3%
select_random_ranges 132.17/s 132.02/s -0.2%
table_scan_postgres 11.60/s 11.87/s +2.3%
types_table_scan_postgres 5.46/s 5.54/s +1.4%

Copy link
Contributor

github-actions bot commented Jan 7, 2025

Main PR
Total 42090 42090
Successful 15358 15377
Failures 26732 26713
Partial Successes1 5238 5238
Main PR
Successful 36.4885% 36.5336%
Failures 63.5115% 63.4664%

${\color{lightgreen}Progressions (19)}$

alter_table

QUERY: ALTER TABLE not_owned_by_me OWNER TO regress_test_not_me;

cluster

QUERY: ALTER TABLE clstr_1 OWNER TO regress_clstr_user;
QUERY: ALTER TABLE clstr_3 OWNER TO regress_clstr_user;
QUERY: ALTER TABLE ptnowner1 OWNER TO regress_ptnowner;
QUERY: ALTER TABLE ptnowner OWNER TO regress_ptnowner;

dependency

QUERY: ALTER TABLE deptest OWNER TO regress_dep_user3;

merge

QUERY: ALTER TABLE target OWNER TO regress_merge_privs;
QUERY: ALTER TABLE source OWNER TO regress_merge_privs;
QUERY: ALTER TABLE target2 OWNER TO regress_merge_no_privs;
QUERY: ALTER TABLE source2 OWNER TO regress_merge_no_privs;

rowsecurity

QUERY: ALTER TABLE t1 OWNER TO regress_rls_alice;

security_label

QUERY: ALTER TABLE seclabel_tbl1 OWNER TO regress_seclabel_user1;
QUERY: ALTER TABLE seclabel_tbl2 OWNER TO regress_seclabel_user2;

tablespace

QUERY: ALTER TABLE testschema.tablespace_acl OWNER TO regress_tablespace_user2;

vacuum

QUERY: ALTER TABLE vacowned_parted OWNER TO regress_vacuum;
QUERY: ALTER TABLE vacowned_part1 OWNER TO regress_vacuum;
QUERY: ALTER TABLE vacowned_parted OWNER TO CURRENT_USER;
QUERY: ALTER TABLE vacowned_parted OWNER TO regress_vacuum;
QUERY: ALTER TABLE vacowned_part1 OWNER TO CURRENT_USER;

Footnotes

  1. These are tests that we're marking as Successful, however they do not match the expected output in some way. This is due to small differences, such as different wording on the error messages, or the column names being incorrect while the data itself is correct.

Copy link
Collaborator

@Hydrocharged Hydrocharged left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

server/ast/no_op.go Show resolved Hide resolved
server/node/no_op.go Show resolved Hide resolved
server/ast/alter_table.go Outdated Show resolved Hide resolved
server/ast/alter_table.go Show resolved Hide resolved
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants