[YSQL] pg backend crash on case involving function, view/table, text search #18879

jasonyb · 2023-08-28T23:36:17Z

Jira Link: DB-7738

Description

The following series of queries crashes the pg backend on master commit abb1f4c and 2.18.1.0; no crash on 2.16.4.0 or 2.14.9.0. The queries are derived from a text search blog post.

CREATE TABLE movies_search (movie_id int, movie_abstract text, movie_abstract_ts tsvector);
INSERT INTO movies_search VALUES (1, 'abstract', to_tsvector('simple', 'abstract'));
create or replace function find_movie(text)
returns setof movies_search as $sql$
  select * from movies_search where to_tsquery('simple',$1) @@ movie_abstract_ts;
$sql$ language sql;
SET yb_debug_report_error_stacktrace = on;
select * from find_movie('Luke');

Original repro:

CREATE TABLE movies (id int, name text);
INSERT INTO movies VALUES (1, '1');
CREATE TABLE movie_abstracts_en (movie_id int, abstract text);
INSERT INTO movie_abstracts_en VALUES (1, 'abstract');
create or replace view movies_search as
 select *
 from (
    select id as movie_id, name as movie_name from movies
 ) movies
 natural join
 (
    select movie_id, abstract as movie_abstract,
     to_tsvector('simple',abstract) as movie_abstract_ts
    from movie_abstracts_en
 ) movie_abstracts;
create or replace function find_movie(text)
returns setof movies_search as $sql$
 select * from movies_search
 where websearch_to_tsquery('simple',$1)
 @@ movie_abstract_ts;
$sql$ language sql;
SET yb_debug_report_error_stacktrace = on;
select * from find_movie(
            'Luke and Leia and "George Lucas"
            ');

Short stack:

ERROR:  Shutdown connection
    @     0x5575750bd047  yb_errmsg_from_status_data
    @     0x557574b04097  YbGetMasterCatalogVersion
    @     0x557574f12ba9  YBPrepareCacheRefreshIfNeeded
    @     0x557574f0a8c1  PostgresMain
    @     0x557574e4a15e  BackendRun
    @     0x557574e49220  ServerLoop
    @     0x557574e44695  PostmasterMain
    @     0x557574d499cf  PostgresServerProcessMain
    @     0x557574a12e22  main
    @     0x7f0f68980825  __libc_start_main
    @     0x557574a12d39  _start

Long stack:

FATAL:  terminating connection due to administrator command
../../src/yb/yql/pggate/util/ybc_util.cc:338:                                                           @     0x7f7e40755324  YBCGetStackTrace
../../../../../../../src/postgres/src/backend/utils/error/elog.c:1112:                                  @           0xa44984  errmsg
../../../../../../src/postgres/src/backend/tcop/postgres.c:3058:                                        @           0x8e97eb  ProcessInterrupts
../../../../../../src/postgres/src/backend/tcop/postgres.c:600:                                         @           0x8ea07d  ProcessClientReadInterrupt
../../../../../../src/postgres/src/backend/libpq/be-secure.c:151:                                       @           0x77e05f  secure_read
../../../../../../src/postgres/src/backend/libpq/pqcomm.c:1005:                                         @           0x791d67  pq_recvbuf
../../../../../../src/postgres/src/backend/libpq/pqcomm.c:1048:                                         @           0x793231  pq_getbyte
../../../../../../src/postgres/src/backend/tcop/postgres.c:361:                                         @           0x8ea23f  SocketBackend
../../../../../../src/postgres/src/backend/tcop/postgres.c:575:                                         @           0x8ea23f  ReadCommand
../../../../../../src/postgres/src/backend/tcop/postgres.c:5326:                                        @           0x8f0841  PostgresMain
../../../../../../src/postgres/src/backend/postmaster/postmaster.c:4658:                                @           0x85163a  BackendRun
../../../../../../src/postgres/src/backend/postmaster/postmaster.c:4296:                                @           0x85163a  BackendStartup
../../../../../../src/postgres/src/backend/postmaster/postmaster.c:1775:                                @           0x85163a  ServerLoop
../../../../../../src/postgres/src/backend/postmaster/postmaster.c:1431:                                @           0x853ac2  PostmasterMain
../../../../../../src/postgres/src/backend/main/main.c:234:                                             @           0x79c925  PostgresServerProcessMain
    @           0x79c945
    @     0x7f7e3c572d84
    @           0x4a783d
    @ 0xffffffffffffffff

ERROR:  Shutdown connection
../../src/yb/yql/pggate/util/ybc_util.cc:338:                                                           @     0x7f7e40755324  YBCGetStackTrace
../../../../../../../src/postgres/src/backend/utils/error/elog.c:4781:                                  @           0xa47ba4  yb_errmsg_from_status_data
../../../../../../../src/postgres/src/backend/catalog/yb_catalog/yb_catalog_version.c:477:              @           0x59a145  YbGetMasterCatalogVersionFromTable
../../../../../../../src/postgres/src/backend/catalog/yb_catalog/yb_catalog_version.c:58:               @           0x59a145  YbGetMasterCatalogVersion
../../../../../../src/postgres/src/backend/tcop/postgres.c:3877:                                        @           0x8e84fc  YBPrepareCacheRefreshIfNeeded
../../../../../../src/postgres/src/backend/tcop/postgres.c:5403:                                        @           0x8f05f2  PostgresMain
../../../../../../src/postgres/src/backend/postmaster/postmaster.c:4658:                                @           0x85163a  BackendRun
../../../../../../src/postgres/src/backend/postmaster/postmaster.c:4296:                                @           0x85163a  BackendStartup
../../../../../../src/postgres/src/backend/postmaster/postmaster.c:1775:                                @           0x85163a  ServerLoop
../../../../../../src/postgres/src/backend/postmaster/postmaster.c:1431:                                @           0x853ac2  PostmasterMain
../../../../../../src/postgres/src/backend/main/main.c:234:                                             @           0x79c925  PostgresServerProcessMain
    @           0x79c945
    @     0x7f7e3c572d84
    @           0x4a783d
    @ 0xffffffffffffffff

Warning: Please confirm that this issue does not contain any sensitive information

I confirm this issue does not contain any sensitive information.

The text was updated successfully, but these errors were encountered:

jasonyb · 2023-08-29T22:26:58Z

Bisected down to commit fdecf5d "[#13541] YSQL: Make yb_enable_expression_pushdown is on by default"

Also tested the repro with SET yb_enable_expression_pushdown = off; and found it passes.

Bisect script:

#!/usr/bin/env bash
set -eux
bin/yb-ctl destroy || true
./yb_build.sh fastdebug --gcc11 --clean-force --sj daemons initdb || exit 125
bin/yb-ctl create
while ! bin/ysqlsh -c ''; do sleep 1; done
bin/ysqlsh -v "ON_ERROR_STOP=1" <<EOT
CREATE TABLE movies_search (movie_id int, movie_abstract text, movie_abstract_ts tsvector);
INSERT INTO movies_search VALUES (1, 'abstract', to_tsvector('simple', 'abstract'));
create or replace function find_movie(text)
returns setof movies_search as \$sql$
  select * from movies_search where to_tsquery('simple',\$1) @@ movie_abstract_ts;
\$sql$ language sql;
SET yb_debug_report_error_stacktrace = on;
select * from find_movie('Luke');
EOT

andrei-mart · 2023-08-29T23:08:52Z

yugabyte=# explain (costs off) select * from movies_search where to_tsquery('simple','Luke') @@ movie_abstract_ts;
                         QUERY PLAN                          
-------------------------------------------------------------
 Seq Scan on movies_search
   Remote Filter: ('''luke'''::tsquery @@ movie_abstract_ts)
(2 rows)

Functions and operators on tsvector data type are unsafe to push down. Seems planner need an exception for that data type.

… thread safe Summary: Operation pushdown invokes Postgres code to evaluate clauses in a multithreaded environment (the TServer). Most text_search variables call the function `getTSCurrentConfig`, which uses global variables for state, so its unsafe in a multithreaded environment. Therefore these functions are only safe to evaluate on the Postgres side, not the TServer side. The fix is to disable pushdown of the text search functions that have unsafe code. ### The Original Issue ### The Github issue found a problem specifically with ```lang=sql returns setof movies_search as $sql$ select * from movies_search where to_tsquery('simple',$1) @@ movie_abstract_ts; $sql$ language sql; select * from find_movie('Luke'); ``` I found that running the query by itself ```lang=sql select * from movies_search where to_tsquery('simple','Luke') @@ movie_abstract_ts; ``` succeeded. It turns out the problem is when the `to_tsquery` is invoked. Within a function, `to_tsquery` is substituted remotely, whereas in the simple query form, `to_tsquery('simple','Luke')` is evaluated as a constant on the PG side and that constant is sent to DocDB. I found that queries of the form: ```lang=sql select * from movies_search where to_tsquery('simple',movie_abstract) @@ movie_abstract_ts ``` would also fail, because this query required that `to_tsquery` would be evaluated on the TServer side. Disabling the pushdown of any function that uses `getTSCurrentConfig` fixed both cases. ## Affected Releases The commit that visibly introduced the regression is fdecf5d / D20292, enabling expression pushdown by default. However, if expression pushdown is enabled manually, it will fail on all releases 2.11 with the expression pushdown framework (2.11+). A backport to 2.14, 2.16, and 2.18 will follow. Jira: DB-7738 Test Plan: ```lang=sh ./yb_build.sh --java-test 'org.yb.pgsql.TestPgRegressTypesString' ./yb_build.sh --java-test 'org.yb.pgsql.TestPgRegressGin' ``` There are some cases in `yb_ybgin_pushdown.sql` and `yb_ybgin_misc.sql` (part of `TestPgRegressGin`) that use a remote filter on text search values. Those cases are still allowed to use remote filters because they are safe functions to push down. Reviewers: amartsinchyk, jason Reviewed By: amartsinchyk Subscribers: kannan, jason, yql Differential Revision: https://phorge.dev.yugabyte.com/D28198

… are not thread safe Summary: Operation pushdown invokes Postgres code to evaluate clauses in a multithreaded environment (the TServer). Most text_search variables call the function `getTSCurrentConfig`, which uses global variables for state, so its unsafe in a multithreaded environment. Therefore these functions are only safe to evaluate on the Postgres side, not the TServer side. The fix is to disable pushdown of the text search functions that have unsafe code. ### The Original Issue ### The Github issue found a problem specifically with ```lang=sql returns setof movies_search as $sql$ select * from movies_search where to_tsquery('simple',$1) @@ movie_abstract_ts; $sql$ language sql; select * from find_movie('Luke'); ``` I found that running the query by itself ```lang=sql select * from movies_search where to_tsquery('simple','Luke') @@ movie_abstract_ts; ``` succeeded. It turns out the problem is when the `to_tsquery` is invoked. Within a function, `to_tsquery` is substituted remotely, whereas in the simple query form, `to_tsquery('simple','Luke')` is evaluated as a constant on the PG side and that constant is sent to DocDB. I found that queries of the form: ```lang=sql select * from movies_search where to_tsquery('simple',movie_abstract) @@ movie_abstract_ts ``` would also fail, because this query required that `to_tsquery` would be evaluated on the TServer side. Disabling the pushdown of any function that uses `getTSCurrentConfig` fixed both cases. ## Affected Releases The commit that visibly introduced the regression is fdecf5d / D20292, enabling expression pushdown by default. However, if expression pushdown is enabled manually, it will fail on all releases 2.11 with the expression pushdown framework (2.11+). A backport to 2.14, 2.16, and 2.18 will follow. Jira: DB-7738 Test Plan: ```lang=sh ./yb_build.sh --java-test 'org.yb.pgsql.TestPgRegressTypesString' ./yb_build.sh --java-test 'org.yb.pgsql.TestPgRegressGin' ``` There are some cases in `yb_ybgin_pushdown.sql` and `yb_ybgin_misc.sql` (part of `TestPgRegressGin`) that use a remote filter on text search values. Those cases are still allowed to use remote filters because they are safe functions to push down. Reviewers: amartsinchyk, jason Reviewed By: amartsinchyk Subscribers: kannan, jason, yql Differential Revision: https://phorge.dev.yugabyte.com/D28198

…ns that are not thread safe Summary: Original commit: 4824553 / D28198 Changes from the original commit: added `SET yb_enable_expression_pushdown = TRUE` to the beginning of the new regress test, because expression pushdown was not yet enabled by default. Operation pushdown invokes Postgres code to evaluate clauses in a multithreaded environment (the TServer). Most text_search variables call the function `getTSCurrentConfig`, which uses global variables for state, so its unsafe in a multithreaded environment. Therefore these functions are only safe to evaluate on the Postgres side, not the TServer side. The fix is to disable pushdown of the text search functions that have unsafe code. === The Original Issue The Github issue found a problem specifically with ```lang=sql returns setof movies_search as $sql$ select * from movies_search where to_tsquery('simple',$1) @@ movie_abstract_ts; $sql$ language sql; select * from find_movie('Luke'); ``` I found that running the query by itself ```lang=sql select * from movies_search where to_tsquery('simple','Luke') @@ movie_abstract_ts; ``` succeeded. It turns out the problem is when the `to_tsquery` is invoked. Within a function, `to_tsquery` is substituted remotely, whereas in the simple query form, `to_tsquery('simple','Luke')` is evaluated as a constant on the PG side and that constant is sent to DocDB. I found that queries of the form: ```lang=sql select * from movies_search where to_tsquery('simple',movie_abstract) @@ movie_abstract_ts ``` would also fail, because this query required that `to_tsquery` would be evaluated on the TServer side. Disabling the pushdown of any function that uses `getTSCurrentConfig` fixed both cases. === Affected Releases The commit that visibly introduced the regression is fdecf5d / D20292, enabling expression pushdown by default. However, if expression pushdown is enabled manually, it will fail on all releases 2.11 with the expression pushdown framework (2.11+). Jira: DB-7738 Test Plan: ```lang=sh ./yb_build.sh --java-test 'org.yb.pgsql.TestPgRegressTypesString' ./yb_build.sh --java-test 'org.yb.pgsql.TestPgRegressGin' ``` Reviewers: amartsinchyk, jason Reviewed By: jason Subscribers: yql Differential Revision: https://phorge.dev.yugabyte.com/D28305

…ns that are not thread safe Summary: Original Commit: 4824553 / D28198 Operation pushdown invokes Postgres code to evaluate clauses in a multithreaded environment (the TServer). Most text_search variables call the function `getTSCurrentConfig`, which uses global variables for state, so its unsafe in a multithreaded environment. Therefore these functions are only safe to evaluate on the Postgres side, not the TServer side. The fix is to disable pushdown of the text search functions that have unsafe code. == The Original Issue The Github issue found a problem specifically with ```lang=sql returns setof movies_search as $sql$ select * from movies_search where to_tsquery('simple',$1) @@ movie_abstract_ts; $sql$ language sql; select * from find_movie('Luke'); ``` I found that running the query by itself ```lang=sql select * from movies_search where to_tsquery('simple','Luke') @@ movie_abstract_ts; ``` succeeded. It turns out the problem is when the `to_tsquery` is invoked. Within a function, `to_tsquery` is substituted remotely, whereas in the simple query form, `to_tsquery('simple','Luke')` is evaluated as a constant on the PG side and that constant is sent to DocDB. I found that queries of the form: ```lang=sql select * from movies_search where to_tsquery('simple',movie_abstract) @@ movie_abstract_ts ``` would also fail, because this query required that `to_tsquery` would be evaluated on the TServer side. Disabling the pushdown of any function that uses `getTSCurrentConfig` fixed both cases. == Affected Releases The commit that visibly introduced the regression is fdecf5d / D20292, enabling expression pushdown by default. However, if expression pushdown is enabled manually, it will fail on all releases 2.11 with the expression pushdown framework (2.11+). Jira: DB-7738 Test Plan: ```lang=sh ./yb_build.sh --java-test 'org.yb.pgsql.TestPgRegressTypesString' ./yb_build.sh --java-test 'org.yb.pgsql.TestPgRegressGin' ``` There are some cases in `yb_ybgin_pushdown.sql` and `yb_ybgin_misc.sql` (part of `TestPgRegressGin`) that use a remote filter on text search values. Those cases are still allowed to use remote filters because they are safe functions to push down. Reviewers: amartsinchyk, jason Reviewed By: jason Subscribers: yql Differential Revision: https://phorge.dev.yugabyte.com/D28303

…ns that are not thread safe Summary: Original Commit: 4824553 / D28198 Changes from the original commit: added `SET yb_enable_expression_pushdown = TRUE` to the beginning of the new regress test, because expression pushdown was not yet enabled by default. Operation pushdown invokes Postgres code to evaluate clauses in a multithreaded environment (the TServer). Most text_search variables call the function `getTSCurrentConfig`, which uses global variables for state, so its unsafe in a multithreaded environment. Therefore these functions are only safe to evaluate on the Postgres side, not the TServer side. The fix is to disable pushdown of the text search functions that have unsafe code. == Original Issue The Github issue found a problem specifically with ```lang=sql returns setof movies_search as $sql$ select * from movies_search where to_tsquery('simple',$1) @@ movie_abstract_ts; $sql$ language sql; select * from find_movie('Luke'); ``` I found that running the query by itself ```lang=sql select * from movies_search where to_tsquery('simple','Luke') @@ movie_abstract_ts; ``` succeeded. It turns out the problem is when the `to_tsquery` is invoked. Within a function, `to_tsquery` is substituted remotely, whereas in the simple query form, `to_tsquery('simple','Luke')` is evaluated as a constant on the PG side and that constant is sent to DocDB. I found that queries of the form: ```lang=sql select * from movies_search where to_tsquery('simple',movie_abstract) @@ movie_abstract_ts ``` would also fail, because this query required that `to_tsquery` would be evaluated on the TServer side. Disabling the pushdown of any function that uses `getTSCurrentConfig` fixed both cases. == Affected Releases The commit that visibly introduced the regression is fdecf5d / D20292, enabling expression pushdown by default. However, if expression pushdown is enabled manually, it will fail on all releases 2.11 with the expression pushdown framework (2.11+). Jira: DB-7738 Test Plan: ```lang=sh ./yb_build.sh --java-test 'org.yb.pgsql.TestPgRegressTypesString' ./yb_build.sh --java-test 'org.yb.pgsql.TestPgRegressGin' ``` There are some cases in `yb_ybgin_pushdown.sql` and `yb_ybgin_misc.sql` (part of `TestPgRegressGin`) that use a remote filter on text search values. Those cases are still allowed to use remote filters because they are safe functions to push down. Reviewers: amartsinchyk, jason Reviewed By: jason Subscribers: yql Differential Revision: https://phorge.dev.yugabyte.com/D28302

jasonyb added kind/bug This issue is a bug area/ysql Yugabyte SQL (YSQL) status/awaiting-triage Issue awaiting triage labels Aug 28, 2023

jasonyb changed the title ~~[YSQL] pg backend crash on case involving function, view, text search~~ [YSQL] pg backend crash on case involving function, view/table, text search Aug 28, 2023

jasonyb added this to Backlog in YSQL via automation Aug 28, 2023

yugabyte-ci added the priority/medium Medium priority issue label Aug 28, 2023

sushantrmishra moved this from Backlog to To do in YSQL Aug 29, 2023

sushantrmishra removed the status/awaiting-triage Issue awaiting triage label Aug 29, 2023

sushantrmishra assigned jasonyb Aug 29, 2023

sushantrmishra assigned timothy-e and unassigned jasonyb Aug 29, 2023

timothy-e closed this as completed Sep 7, 2023

YSQL automation moved this from To do to Done Sep 7, 2023

yugabyte-ci reopened this Sep 11, 2023

YSQL automation moved this from Done to In progress Sep 11, 2023

yugabyte-ci closed this as completed Sep 14, 2023

YSQL automation moved this from In progress to Done Sep 14, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[YSQL] pg backend crash on case involving function, view/table, text search #18879

[YSQL] pg backend crash on case involving function, view/table, text search #18879

jasonyb commented Aug 28, 2023 •

edited by jira bot

jasonyb commented Aug 29, 2023

andrei-mart commented Aug 29, 2023

[YSQL] pg backend crash on case involving function, view/table, text search #18879

[YSQL] pg backend crash on case involving function, view/table, text search #18879

Comments

jasonyb commented Aug 28, 2023 • edited by jira bot

Description

Warning: Please confirm that this issue does not contain any sensitive information

jasonyb commented Aug 29, 2023

andrei-mart commented Aug 29, 2023

jasonyb commented Aug 28, 2023 •

edited by jira bot