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

Set PG dialect While Running FKEY checks #518

Open
wants to merge 3 commits into
base: BABEL_5_X_DEV__PG_17_X
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .github/workflows/main.yml
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ jobs:
run: |
sudo apt update --fix-missing -y
sudo apt install -y libipc-run-perl
sudo apt-get install -y libreadline-dev
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Was getting build error

checking for library containing pthread_barrier_wait... none required
checking for library containing readline... no
configure: error: readline library not found
If you have readline already installed, see config.log for details on the
failure.  It is possible the compiler isn't looking in the proper directory.
Use --without-readline to disable readline support.

Copy link
Contributor

Choose a reason for hiding this comment

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

Probably better to configure engine with --without-readline.

- name: build-postgres
run: |
./configure --with-icu --enable-cassert --enable-tap-tests
Expand Down
24 changes: 12 additions & 12 deletions src/backend/utils/adt/ri_triggers.c
Original file line number Diff line number Diff line change
Expand Up @@ -430,11 +430,11 @@ RI_FKey_check(TriggerData *trigdata)
* referenced side. The reason is that our snapshot must be fresh in
* order for the hack in find_inheritance_children() to work.
*/
ri_PerformCheck(riinfo, &qkey, qplan,
RUN_AS_PSQL(ri_PerformCheck(riinfo, &qkey, qplan,
Copy link
Contributor

Choose a reason for hiding this comment

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

We need to change RUN_AS_PSQL macro a bit.
We should directly set sql_dialect using its global variable rather than set_config.

fk_rel, pk_rel,
NULL, newslot,
pk_rel->rd_rel->relkind == RELKIND_PARTITIONED_TABLE,
SPI_OK_SELECT);
SPI_OK_SELECT));

if (SPI_finish() != SPI_OK_FINISH)
elog(ERROR, "SPI_finish failed");
Expand Down Expand Up @@ -557,11 +557,11 @@ ri_Check_Pk_Match(Relation pk_rel, Relation fk_rel,
/*
* We have a plan now. Run it.
*/
result = ri_PerformCheck(riinfo, &qkey, qplan,
RUN_AS_PSQL(result = ri_PerformCheck(riinfo, &qkey, qplan,
fk_rel, pk_rel,
oldslot, NULL,
true, /* treat like update */
SPI_OK_SELECT);
SPI_OK_SELECT));

if (SPI_finish() != SPI_OK_FINISH)
elog(ERROR, "SPI_finish failed");
Expand Down Expand Up @@ -749,11 +749,11 @@ ri_restrict(TriggerData *trigdata, bool is_no_action)
/*
* We have a plan now. Run it to check for existing references.
*/
ri_PerformCheck(riinfo, &qkey, qplan,
RUN_AS_PSQL(ri_PerformCheck(riinfo, &qkey, qplan,
fk_rel, pk_rel,
oldslot, NULL,
true, /* must detect new rows */
SPI_OK_SELECT);
SPI_OK_SELECT));

if (SPI_finish() != SPI_OK_FINISH)
elog(ERROR, "SPI_finish failed");
Expand Down Expand Up @@ -855,11 +855,11 @@ RI_FKey_cascade_del(PG_FUNCTION_ARGS)
* We have a plan now. Build up the arguments from the key values in the
* deleted PK tuple and delete the referencing rows
*/
ri_PerformCheck(riinfo, &qkey, qplan,
RUN_AS_PSQL(ri_PerformCheck(riinfo, &qkey, qplan,
fk_rel, pk_rel,
oldslot, NULL,
true, /* must detect new rows */
SPI_OK_DELETE);
SPI_OK_DELETE));

if (SPI_finish() != SPI_OK_FINISH)
elog(ERROR, "SPI_finish failed");
Expand Down Expand Up @@ -976,11 +976,11 @@ RI_FKey_cascade_upd(PG_FUNCTION_ARGS)
/*
* We have a plan now. Run it to update the existing references.
*/
ri_PerformCheck(riinfo, &qkey, qplan,
RUN_AS_PSQL(ri_PerformCheck(riinfo, &qkey, qplan,
fk_rel, pk_rel,
oldslot, newslot,
true, /* must detect new rows */
SPI_OK_UPDATE);
SPI_OK_UPDATE));

if (SPI_finish() != SPI_OK_FINISH)
elog(ERROR, "SPI_finish failed");
Expand Down Expand Up @@ -1208,11 +1208,11 @@ ri_set(TriggerData *trigdata, bool is_set_null, int tgkind)
/*
* We have a plan now. Run it to update the existing references.
*/
ri_PerformCheck(riinfo, &qkey, qplan,
RUN_AS_PSQL(ri_PerformCheck(riinfo, &qkey, qplan,
fk_rel, pk_rel,
oldslot, NULL,
true, /* must detect new rows */
SPI_OK_UPDATE);
SPI_OK_UPDATE));

if (SPI_finish() != SPI_OK_FINISH)
elog(ERROR, "SPI_finish failed");
Expand Down
3 changes: 1 addition & 2 deletions src/backend/utils/misc/queryenvironment.c
Original file line number Diff line number Diff line change
Expand Up @@ -1783,8 +1783,7 @@ bool IsTsqlTableVariable(Relation relation)
{
return sql_dialect == SQL_DIALECT_TSQL
&& relation
&& relation->rd_rel->relpersistence == RELPERSISTENCE_TEMP
&& relation->rd_rel->relname.data
&& relation->rd_rel->relpersistence == RELPERSISTENCE_TEMP
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Had to fix this due to following build error

queryenvironment.c: In function ‘IsTsqlTableVariable’:
queryenvironment.c:1787:17: error: the comparison will always evaluate as ‘true’ for the address of ‘data’ will never be NULL [-Werror=address]
 1787 |                 && relation->rd_rel->relname.data
      |                 ^~
In file included from ../../../../src/include/postgres.h:45,
                 from queryenvironment.c:23:
../../../../src/include/c.h:742:25: note: ‘data’ declared here
  742 |         char            data[NAMEDATALEN];
      | 

ref

Copy link
Contributor

Choose a reason for hiding this comment

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

I think the original check was aimed at being NULL safe, so we should still add

&& relation->rd_rel->relname

Although I am not sure how relname would ever be NULL, but still good to have.

&& strlen(relation->rd_rel->relname.data) >= 1
&& relation->rd_rel->relname.data[0] == '@';
}
Expand Down
Loading