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

Move INSERT query of repack.repack_trigger to C #368

Merged
merged 2 commits into from
Oct 27, 2023

Conversation

za-arthur
Copy link
Collaborator

Making INSERT query in C prevents SQL injections, which are possible if an INSERT query is passed as an argument.

Here is an example with "old" repack.repack_trigger:

CREATE EXTENSION pg_repack;
CREATE TABLE public.t1 (a int);
CREATE TABLE public.t2 (b varchar);
INSERT INTO public.t2 VALUES (current_user);
CREATE TRIGGER repack_trigger AFTER INSERT ON public.t1
FOR EACH ROW EXECUTE PROCEDURE repack.repack_trigger('INSERT INTO public.t2 VALUES (current_user)');
INSERT INTO public.t1 VALUES (1);
SELECT b FROM public.t2;
  b 
----------
 bob
 postgres 

It is possible to move repack.get_index_columns to C too. But list of columns is passed as an argument as for now for the sake of simplicity.

Additionally this commit adds funtions:

  • repack.create_index_type
  • repack.create_log_table
  • repack.create_table

Another change is that tablespace_orig considers default tablespace of a database (issues #363, #305)

Making INSERT query in C prevents SQL injections, which are
possible if an INSERT query is passed as an argument.

Additionally this commit adds funtions:
- repack.create_index_type
- repack.create_log_table
- repack.create_table

Another change is that tablespace_orig considers default tablespace
of a database.
Copy link
Member

@dvarrazzo dvarrazzo left a comment

Choose a reason for hiding this comment

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

It looks good to me, apart from the suggestion to add a comment on the change of quoting. Thank you very much for this work 💜

lib/pg_repack.sql.in Show resolved Hide resolved
lib/pg_repack.sql.in Outdated Show resolved Hide resolved
regress/sql/trigger.sql Show resolved Hide resolved
lib/repack.c Show resolved Hide resolved
@za-arthur
Copy link
Collaborator Author

Thank you @dvarrazzo for the review. I pushed my final commit. You already approved the PR, but I want to make sure that you don't mind if I will merge the PR in the current state?

@za-arthur za-arthur merged commit 4c55e6e into reorg:master Oct 27, 2023
14 checks passed
@za-arthur za-arthur deleted the create_table branch October 27, 2023 12:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants