Skip to content

pg_ext: fix to_regclass(), ::regclass and pg_settings#8511

Merged
fantix merged 18 commits intomasterfrom
sql-to_regclass
Apr 4, 2025
Merged

pg_ext: fix to_regclass(), ::regclass and pg_settings#8511
fantix merged 18 commits intomasterfrom
sql-to_regclass

Conversation

@fantix
Copy link
Member

@fantix fantix commented Mar 26, 2025

Fixes #8508
Refs #8488

  • Implement frontend-aware pg_settings
  • Implement split_identifier_string()
  • Implement current_schemas() function in the backend
  • Fix SQL implementation of to_regclass()
  • Fix regclass typecasting
  • Drop static evaluation of replaced functions
  • Add test if not yet

@fantix fantix added the to-backport-6.x PRs that *should* be backported to 6.x label Mar 26, 2025
@fantix fantix force-pushed the sql-to_regclass branch 2 times, most recently from 2b58bbd to aa51219 Compare March 27, 2025 19:58
@fantix fantix requested review from aljazerzen and elprans March 27, 2025 20:52
@fantix fantix marked this pull request as ready for review March 27, 2025 20:52
Copy link
Contributor

@aljazerzen aljazerzen left a comment

Choose a reason for hiding this comment

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

Kudos fantix! Was this translated from postgres's C implementation?

@fantix
Copy link
Member Author

fantix commented Mar 28, 2025

Yeah, it's from C. Unfortunately, they don't have SQL interfaces.

Copy link
Member

@elprans elprans left a comment

Choose a reason for hiding this comment

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

LGTM, but please add some tests before merging.

@fantix
Copy link
Member Author

fantix commented Mar 31, 2025

Right, I noticed that in-transaction settings are not applied during writing tests, will also have to fix that.

@aljazerzen
Copy link
Contributor

FYI: some of our sql test classes use transactions and some don't, that might be interfering.

(I'm not really sure what you mean fantix, so I don't know if this is relevant)

CREATE CONSTRAINT TRIGGER _edgecon_state_local_reset
AFTER INSERT ON _edgecon_state
DEFERRABLE INITIALLY DEFERRED
FOR EACH ROW EXECUTE FUNCTION edgedb._clear_fe_local_sql_settings();
Copy link
Member Author

Choose a reason for hiding this comment

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

Ah, this will be tricky for edb.pgsql.patches to patch up, let me think of a workaround

Copy link
Member Author

Choose a reason for hiding this comment

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

Fixed, see commit below.

@fantix fantix merged commit 1d58f10 into master Apr 4, 2025
24 checks passed
@fantix fantix deleted the sql-to_regclass branch April 4, 2025 20:28
@msullivan msullivan added backported-6.x PRs that *have* been backported to 6.x to-backport-6.x PRs that *should* be backported to 6.x and removed to-backport-6.x PRs that *should* be backported to 6.x backported-6.x PRs that *have* been backported to 6.x labels Apr 7, 2025
fantix added a commit that referenced this pull request Apr 7, 2025
* Implement frontend-aware `pg_settings` and `pg_show_all_settings`
* Implement `current_schemas()` function in the backend
* Fix SQL implementation of `to_regclass()` and typecasting
* Fix SHOW ALL command
* Re-implement SHOW command with backend SQL
fantix added a commit that referenced this pull request Apr 7, 2025
* Implement frontend-aware `pg_settings` and `pg_show_all_settings`
* Implement `current_schemas()` function in the backend
* Fix SQL implementation of `to_regclass()` and typecasting
* Fix SHOW ALL command
* Re-implement SHOW command with backend SQL
@msullivan msullivan added backported-6.x PRs that *have* been backported to 6.x and removed to-backport-6.x PRs that *should* be backported to 6.x labels Apr 8, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backported-6.x PRs that *have* been backported to 6.x

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Gel 6.4 Metabase syncing error | Upgraded from Gel 6.2

4 participants