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

Avoid using backslash in SQL string literals #7669

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

Green-Chan
Copy link
Contributor

If parameter standard_conforming_strings is off, backslashes in ordinary string literals ('...') are treated as escape characters causing problems. In particular CREATE EXTENSION citus; is failing when standard_conforming_strings=off:

postgres=# CREATE EXTENSION citus;
WARNING:  nonstandard use of escape in a string literal
LINE 6: IF substring(current_Setting('server_version'), '\d+')::int ...
                                                        ^
HINT:  Use the escape string syntax for escapes, e.g., E'\r\n'.
QUERY:  
BEGIN
-- from version 12 and up we have support for tableam's if installed on pg11 we can't
-- create the objects here. Instead we rely on citus_finish_pg_upgrade to be called by the
-- user instead to add the missing objects
IF substring(current_Setting('server_version'), '\d+')::int >= 12 THEN
  EXECUTE $$
--#include "udfs/columnar_handler/10.0-1.sql"
CREATE OR REPLACE FUNCTION columnar.columnar_handler(internal)
    RETURNS table_am_handler
    LANGUAGE C
AS '$libdir/citus_columnar', 'columnar_handler';
COMMENT ON FUNCTION columnar.columnar_handler(internal)
    IS 'internal function returning the handler for columnar tables';
-- postgres 11.8 does not support the syntax for table am, also it is seemingly trying
-- to parse the upgrade file and erroring on unknown syntax.
-- normally this section would not execute on postgres 11 anyway. To trick it to pass on
-- 11.8 we wrap the statement in a plpgsql block together with an EXECUTE. This is valid
-- syntax on 11.8 and will execute correctly in 12
DO $create_table_am$
BEGIN
  EXECUTE 'CREATE ACCESS METHOD columnar TYPE TABLE HANDLER columnar.columnar_handler';
END $create_table_am$;
--#include "udfs/alter_columnar_table_set/10.0-1.sql"
CREATE OR REPLACE FUNCTION pg_catalog.alter_columnar_table_set(
    table_name regclass,
    chunk_group_row_limit int DEFAULT NULL,
    stripe_row_limit int DEFAULT NULL,
    compression name DEFAULT null,
    compression_level int DEFAULT NULL)
    RETURNS void
    LANGUAGE C
AS '$libdir/citus_columnar', 'alter_columnar_table_set';
COMMENT ON FUNCTION pg_catalog.alter_columnar_table_set(
    table_name regclass,
    chunk_group_row_limit int,
    stripe_row_limit int,
    compression name,
    compression_level int)
IS 'set one or more options on a columnar table, when set to NULL no change is made';
--#include "udfs/alter_columnar_table_reset/10.0-1.sql"
CREATE OR REPLACE FUNCTION pg_catalog.alter_columnar_table_reset(
    table_name regclass,
    chunk_group_row_limit bool DEFAULT false,
    stripe_row_limit bool DEFAULT false,
    compression bool DEFAULT false,
    compression_level bool DEFAULT false)
    RETURNS void
    LANGUAGE C
AS '$libdir/citus_columnar', 'alter_columnar_table_reset';
COMMENT ON FUNCTION pg_catalog.alter_columnar_table_reset(
    table_name regclass,
    chunk_group_row_limit bool,
    stripe_row_limit bool,
    compression bool,
    compression_level bool)
IS 'reset on or more options on a columnar table to the system defaults';
  $$;
END IF;
END
WARNING:  nonstandard use of escape in a string literal
LINE 1: substring(current_Setting('server_version'), '\d+')::int >= ...
                                                     ^
HINT:  Use the escape string syntax for escapes, e.g., E'\r\n'.
QUERY:  substring(current_Setting('server_version'), '\d+')::int >= 12
ERROR:  null value in column "objid" of relation "pg_depend" violates not-null constraint
DETAIL:  Failing row contains (2601, null, 0, 1259, 16387, 0, n).
CONTEXT:  SQL statement "INSERT INTO pg_depend
  WITH columnar_schema_members(relid) AS (
    SELECT pg_class.oid AS relid FROM pg_class
      WHERE relnamespace =
            COALESCE(
        (SELECT pg_namespace.oid FROM pg_namespace WHERE nspname = 'columnar_internal'),
        (SELECT pg_namespace.oid FROM pg_namespace WHERE nspname = 'columnar')
     )
        AND relname IN ('chunk',
                        'chunk_group',
                        'chunk_group_pkey',
                        'chunk_pkey',
                        'options',
                        'options_pkey',
                        'storageid_seq',
                        'stripe',
                        'stripe_first_row_number_idx',
                        'stripe_pkey')
  )
  SELECT -- Define a dependency edge from "columnar table access method" ..
         'pg_am'::regclass::oid as classid,
         (select oid from pg_am where amname = 'columnar') as objid,
         0 as objsubid,
         -- ... to each object that is registered to pg_class and that lives
         -- in "columnar" schema. That contains catalog tables, indexes
         -- created on them and the sequences created in "columnar" schema.
         --
         -- Given the possibility of user might have created their own objects
         -- in columnar schema, we explicitly specify list of objects that we
         -- are interested in.
         'pg_class'::regclass::oid as refclassid,
         columnar_schema_members.relid as refobjid,
         0 as refobjsubid,
         'n' as deptype
  FROM columnar_schema_members
  -- Avoid inserting duplicate entries into pg_depend.
  EXCEPT TABLE pg_depend"
PL/pgSQL function columnar_internal.columnar_ensure_am_depends_catalog() line 3 at SQL statement

This PR eliminates usages of backslashes in SQL string literals, therefore allowing using Citus when standard_conforming_strings=off.

@Green-Chan
Copy link
Contributor Author

This PR could probably be connected to issues #6968 and #5985, but they seem too old to me to ask the authors if it helps.

@Green-Chan
Copy link
Contributor Author

I haven't touched string literals in tests because citus is not tested with standard_conforming_strings=off anyway. But it could also be done

@@ -474,7 +474,7 @@ PendingWorkerTransactionList(MultiConnection *connection)
int32 coordinatorId = GetLocalGroupId();

appendStringInfo(command, "SELECT gid FROM pg_prepared_xacts "
"WHERE gid LIKE 'citus\\_%d\\_%%' and database = current_database()",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Any idea why %% is used here? Wouldn't % do the same? We could fix it while we are here

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant