Adds null check for node in HasRangeTableRef #7604

gurkanindibay · 2024-05-20T12:47:28Z

DESCRIPTION: Adds null check for node in HasRangeTableRef to prevent errors

When executing the query below, users encountered an error due to a null Node object. This PR adds a null check to handle this error.

Query:

select
    ct.conname as constraint_name,
    a.attname as column_name,
    fc.relname as foreign_table_name,
    fns.nspname as foreign_table_schema,
    fa.attname as foreign_column_name
from
    (SELECT ct.conname, ct.conrelid, ct.confrelid, ct.conkey, ct.contype,
ct.confkey, generate_subscripts(ct.conkey, 1) AS s
       FROM pg_constraint ct
    ) AS ct
    inner join pg_class c on c.oid=ct.conrelid
    inner join pg_namespace ns on c.relnamespace=ns.oid
    inner join pg_attribute a on a.attrelid=ct.conrelid and a.attnum =
ct.conkey[ct.s]
    left join pg_class fc on fc.oid=ct.confrelid
    left join pg_namespace fns on fc.relnamespace=fns.oid
    left join pg_attribute fa on fa.attrelid=ct.confrelid and fa.attnum =
ct.confkey[ct.s]
where
    ct.contype='f'
    and c.relname='table1'
    and ns.nspname='schemauser'
order by
    fns.nspname, fc.relname, a.attnum
;

Error:

#0  HasRangeTableRef (node=0x0, varno=varno@entry=0x7ffe18cc3674) at worker/worker_shard_visibility.c:507
507             if (IsA(node, RangeTblRef))
#0  HasRangeTableRef (node=0x0, varno=varno@entry=0x7ffe18cc3674) at worker/worker_shard_visibility.c:507
#1  0x0000561b0aae390e in expression_tree_walker_impl (node=0x561b0d19cc78, walker=walker@entry=0x7f2a73249f0a <HasRangeTableRef>, context=0x7ffe18cc3674)
    at nodeFuncs.c:2091
#2  0x00007f2a73249f26 in HasRangeTableRef (node=<optimized out>, varno=<optimized out>) at worker/worker_shard_visibility.c:513
#3  0x0000561b0aae3e09 in expression_tree_walker_impl (node=0x561b0d19cd68, walker=walker@entry=0x7f2a73249f0a <HasRangeTableRef>, context=context@entry=0x7ffe18cc3674)
    at nodeFuncs.c:2405
#4  0x0000561b0aae3945 in expression_tree_walker_impl (node=0x561b0d19d0f8, walker=walker@entry=0x7f2a73249f0a <HasRangeTableRef>, context=0x7ffe18cc3674)
    at nodeFuncs.c:2111
#5  0x00007f2a73249f26 in HasRangeTableRef (node=<optimized out>, varno=<optimized out>) at worker/worker_shard_visibility.c:513
#6  0x0000561b0aae3e09 in expression_tree_walker_impl (node=0x561b0d19cb38, walker=walker@entry=0x7f2a73249f0a <HasRangeTableRef>, context=context@entry=0x7ffe18cc3674)
    at nodeFuncs.c:2405
#7  0x0000561b0aae396d in expression_tree_walker_impl (node=0x561b0d19d198, walker=walker@entry=0x7f2a73249f0a <HasRangeTableRef>, context=0x7ffe18cc3674)
    at nodeFuncs.c:2127
#8  0x00007f2a73249f26 in HasRangeTableRef (node=<optimized out>, varno=<optimized out>) at worker/worker_shard_visibility.c:513
#9  0x0000561b0aae3ef7 in expression_tree_walker_impl (node=0x561b0d183e88, walker=walker@entry=0x7f2a73249f0a <HasRangeTableRef>, context=0x7ffe18cc3674)
    at nodeFuncs.c:2464
#10 0x00007f2a73249f26 in HasRangeTableRef (node=<optimized out>, varno=<optimized out>) at worker/worker_shard_visibility.c:513
#11 0x0000561b0aae3ed3 in expression_tree_walker_impl (node=0x561b0d184278, walker=walker@entry=0x7f2a73249f0a <HasRangeTableRef>, context=0x7ffe18cc3674)
    at nodeFuncs.c:2460
#12 0x00007f2a73249f26 in HasRangeTableRef (node=<optimized out>, varno=<optimized out>) at worker/worker_shard_visibility.c:513
#13 0x0000561b0aae3ed3 in expression_tree_walker_impl (node=0x561b0d184668, walker=walker@entry=0x7f2a73249f0a <HasRangeTableRef>, context=0x7ffe18cc3674)
    at nodeFuncs.c:2460
#14 0x00007f2a73249f26 in HasRangeTableRef (node=<optimized out>, varno=<optimized out>) at worker/worker_shard_visibility.c:513
#15 0x0000561b0aae3ed3 in expression_tree_walker_impl (node=0x561b0d184f68, walker=walker@entry=0x7f2a73249f0a <HasRangeTableRef>, context=0x7ffe18cc3674)
    at nodeFuncs.c:2460
#16 0x00007f2a73249f26 in HasRangeTableRef (node=<optimized out>, varno=<optimized out>) at worker/worker_shard_visibility.c:513
#17 0x0000561b0aae3e09 in expression_tree_walker_impl (node=0x7f2a68010148, walker=walker@entry=0x7f2a73249f0a <HasRangeTableRef>, context=context@entry=0x7ffe18cc3674)
    at nodeFuncs.c:2405
#18 0x00007f2a7324a0eb in FilterShardsFromPgclass (node=node@entry=0x561b0d185de8, context=context@entry=0x0) at worker/worker_shard_visibility.c:464
#19 0x00007f2a7324a5ff in HideShardsFromSomeApplications (query=query@entry=0x561b0d185de8) at worker/worker_shard_visibility.c:294
#20 0x00007f2a731ed7ac in distributed_planner (parse=0x561b0d185de8, 
    query_string=0x561b0d009478 "select\n    ct.conname as constraint_name,\n    a.attname as column_name,\n    fc.relname as foreign_table_name,\n    fns.nspname as foreign_table_schema,\n    fa.attname as foreign_column_name\nfrom\n    (S"..., cursorOptions=<optimized out>, boundParams=0x0) at planner/distributed_planner.c:237
#21 0x00007f2a7311a52a in pgss_planner (parse=0x561b0d185de8, 
    query_string=0x561b0d009478 "select\n    ct.conname as constraint_name,\n    a.attname as column_name,\n    fc.relname as foreign_table_name,\n    fns.nspname as foreign_table_schema,\n    fa.attname as foreign_column_name\nfrom\n    (S"..., cursorOptions=2048, boundParams=0x0) at pg_stat_statements.c:953
#22 0x0000561b0ab65465 in planner (parse=parse@entry=0x561b0d185de8, 
    query_string=query_string@entry=0x561b0d009478 "select\n    ct.conname as constraint_name,\n    a.attname as column_name,\n    fc.relname as foreign_table_name,\n    fns.nspname as foreign_table_schema,\n    fa.attname as foreign_column_name\nfrom\n    (S"..., cursorOptions=cursorOptions@entry=2048, boundParams=boundParams@entry=0x0)
    at planner.c:279
#23 0x0000561b0ac53aa3 in pg_plan_query (querytree=querytree@entry=0x561b0d185de8, 
    query_string=query_string@entry=0x561b0d009478 "select\n    ct.conname as constraint_name,\n    a.attname as column_name,\n    fc.relname as foreign_table_name,\n    fns.nspname as foreign_table_schema,\n    fa.attname as foreign_column_name\nfrom\n    (S"..., cursorOptions=cursorOptions@entry=2048, boundParams=boundParams@entry=0x0)
    at postgres.c:904
#24 0x0000561b0ac53b71 in pg_plan_queries (querytrees=0x7f2a68012878, 
    query_string=query_string@entry=0x561b0d009478 "select\n    ct.conname as constraint_name,\n    a.attname as column_name,\n    fc.relname as foreign_table_name,\n    fns.nspname as foreign_table_schema,\n    fa.attname as foreign_column_name\nfrom\n    (S"..., cursorOptions=cursorOptions@entry=2048, boundParams=boundParams@entry=0x0)
    at postgres.c:996
#25 0x0000561b0ac5408e in exec_simple_query (
    query_string=query_string@entry=0x561b0d009478 "select\n    ct.conname as constraint_name,\n    a.attname as column_name,\n    fc.relname as foreign_table_name,\n    fns.nspname as foreign_table_schema,\n    fa.attname as foreign_column_name\nfrom\n    (S"...) at postgres.c:1193
#26 0x0000561b0ac56116 in PostgresMain (dbname=<optimized out>, username=<optimized out>) at postgres.c:4637
#27 0x0000561b0abab7a7 in BackendRun (port=port@entry=0x561b0d0caf50) at postmaster.c:4464
#28 0x0000561b0abae969 in BackendStartup (port=port@entry=0x561b0d0caf50) at postmaster.c:4192
#29 0x0000561b0abaeaa6 in ServerLoop () at postmaster.c:1782

Fixes #7603

codecov · 2024-05-20T12:52:55Z

Codecov Report

Attention: Patch coverage is 0% with 1 lines in your changes are missing coverage. Please review.

Project coverage is 89.65%. Comparing base (4c110fa) to head (3a1b245).
Report is 31 commits behind head on release-12.1.

Additional details and impacted files

@@               Coverage Diff                @@
##           release-12.1    #7604      +/-   ##
================================================
+ Coverage         89.55%   89.65%   +0.09%     
================================================
  Files               274      274              
  Lines             59479    59574      +95     
  Branches           7415     7435      +20     
================================================
+ Hits              53268    53409     +141     
+ Misses             4082     4036      -46     
  Partials           2129     2129

hanefi · 2024-05-20T14:43:54Z

Can you simplify the query and add a regression test?

gurkanindibay · 2024-05-22T12:15:16Z

Can you simplify the query and add a regression test?

I tried and see that the most lean state of the query is as below. However, in this case, query lose its meaning. Therefore, I kept as is and added a test with a system table and see meaningful results in tests
The most lean state of the query

select
    ct.conname as constraint_name,
    a.attname as column_name,
    fc.relname as foreign_table_name
from
    (SELECT ct.conname, ct.conrelid, ct.confrelid, ct.conkey, ct.contype,
ct.confkey, generate_subscripts(ct.conkey, 1) AS s
       FROM pg_constraint ct
    ) AS ct
    inner join pg_attribute a on a.attrelid=ct.conrelid and a.attnum =
ct.conkey[ct.s]
left outer join pg_class fc on  fc.oid=ct.confrelid

gurkanindibay · 2024-05-23T10:41:05Z

@hanefi thanks for your review. There's a problem in code coverage in patch. I assume it is false positive.

Green-Chan · 2024-05-30T12:01:42Z

The test doesn't actually test anything. I added this test to v12.1.3 and it passed whereas the query itself (being ran in psql) fails

Adds null check for node in HasRangeTableRef

a7aee78

Fixes formatting

c68cfb0

Adds test for the fix

3a1b245

gurkanindibay requested a review from hanefi May 23, 2024 10:29

gurkanindibay changed the base branch from release-12.1 to main May 23, 2024 10:30

gurkanindibay changed the base branch from main to release-12.1 May 23, 2024 10:30

JelteF approved these changes May 27, 2024

View reviewed changes

JelteF mentioned this pull request May 28, 2024

postgres[495645]: segfault at 0 ip 00007f318b17e1f4 sp 00007ffc7f1b15d8 error 4 in citus.so[7f318b0a4000+ee000] likely on CPU 93 (core 1, socket 1) #7603

Closed

gurkanindibay merged commit 4e838a4 into release-12.1 May 28, 2024
124 of 125 checks passed

gurkanindibay deleted the range_table_null_check branch May 28, 2024 05:54

Green-Chan mentioned this pull request May 30, 2024

Fix system_queries test to actually test the problem #7613

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Adds null check for node in HasRangeTableRef #7604

Adds null check for node in HasRangeTableRef #7604

gurkanindibay commented May 20, 2024 •

edited by JelteF

Loading

codecov bot commented May 20, 2024 •

edited

Loading

hanefi commented May 20, 2024

gurkanindibay commented May 22, 2024

gurkanindibay commented May 23, 2024

Green-Chan commented May 30, 2024 •

edited

Loading

Adds null check for node in HasRangeTableRef #7604

Adds null check for node in HasRangeTableRef #7604

Conversation

gurkanindibay commented May 20, 2024 • edited by JelteF Loading

codecov bot commented May 20, 2024 • edited Loading

Codecov Report

hanefi commented May 20, 2024

gurkanindibay commented May 22, 2024

gurkanindibay commented May 23, 2024

Green-Chan commented May 30, 2024 • edited Loading

gurkanindibay commented May 20, 2024 •

edited by JelteF

Loading

codecov bot commented May 20, 2024 •

edited

Loading

Green-Chan commented May 30, 2024 •

edited

Loading