From cb5e9ef589b147c2684bed705717b3f27fd3a8d1 Mon Sep 17 00:00:00 2001 From: Arpan Agrawal Date: Mon, 28 Aug 2023 20:33:17 +0530 Subject: [PATCH] [MERGE PG15] libpq directory and some misc fixes Summary: This diff fixes the following: # Enables the code block in `ClientAuthentication` in `auth.c`, which was disabled during the merge. A minor code modification is done related to the oid now being a regular column. # Enables code blocks in `hba.c` which were disabled during the merge. Previously, a portion of `tokenize_file` was refactored out into a common helper function `tokenize_line` for PG/YB. Subsequent changes in PG upstream led to conflicts during the merge. This diff retains the PG upstream code as it is, along with a YB-specific function `yb_tokenize_line`. It also renames `tokenize_hardcoded` to `yb_tokenize_hardcoded` as it YB-specific. # Adds missing case for `T_BackfillIndexStmt` in the function `ClassifyUtilityCommandAsReadOnly.` # Removes ` Assert(0);` from function `IndexBackfillHeapRangeScan`. YB_TODO comment related to handling the `progress` parameter is already present in the function; hence, assert(0) is not required. With fixes #2, #3, and #4, `CREATE INDEX CONCURRENTLY` works on pg15 branch. Test Plan: Jenkins: rebase: no Reviewers: smishra, neil, jason Reviewed By: jason Subscribers: yql Differential Revision: https://phorge.dev.yugabyte.com/D27860 --- src/postgres/src/backend/catalog/index.c | 1 - src/postgres/src/backend/libpq/auth.c | 9 +- src/postgres/src/backend/libpq/hba.c | 153 +++--------------- src/postgres/src/backend/tcop/utility.c | 1 + .../src/test/regress/expected/yb_pg15.out | 2 + src/postgres/src/test/regress/sql/yb_pg15.sql | 3 + 6 files changed, 30 insertions(+), 139 deletions(-) diff --git a/src/postgres/src/backend/catalog/index.c b/src/postgres/src/backend/catalog/index.c index e2cf65908de9..29a5267d57cb 100644 --- a/src/postgres/src/backend/catalog/index.c +++ b/src/postgres/src/backend/catalog/index.c @@ -3342,7 +3342,6 @@ IndexBackfillHeapRangeScan(Relation table_rel, /* YB_TODO(neil@yugabyte) * - Check for the value of the new flag "progress". */ - Assert(0); return table_rel->rd_tableam->index_build_range_scan(table_rel, index_rel, index_info, diff --git a/src/postgres/src/backend/libpq/auth.c b/src/postgres/src/backend/libpq/auth.c index e0a5fd95652c..40ec05853278 100644 --- a/src/postgres/src/backend/libpq/auth.c +++ b/src/postgres/src/backend/libpq/auth.c @@ -51,6 +51,9 @@ #include "pg_yb_utils.h" #include "yb/yql/pggate/ybc_pggate.h" +/* Yugabyte includes */ +#include "catalog/pg_authid.h" + /*---------------------------------------------------------------- * Global authentication functions *---------------------------------------------------------------- @@ -714,9 +717,6 @@ ClientAuthentication(Port *port) if (ClientAuthentication_hook) (*ClientAuthentication_hook) (port, status); - -#ifdef YB_TODO - /* YB_TODO(neil) Rewrite this. OID is no longer a special column */ /* * If conditions are met, update the role's profile entry. Specific * authentication methods are isolated from profile handling. @@ -732,7 +732,7 @@ ClientAuthentication(Port *port) roleTuple = SearchSysCache1(AUTHNAME, PointerGetDatum(port->user_name)); if (HeapTupleIsValid(roleTuple)) { - roleid = HeapTupleGetOid(roleTuple); + roleid = ((Form_pg_authid) GETSTRUCT(roleTuple))->oid; profileTuple = yb_get_role_profile_tuple_by_role_oid(roleid); if (HeapTupleIsValid(profileTuple)) { @@ -760,7 +760,6 @@ ClientAuthentication(Port *port) } return; } -#endif if (status == STATUS_OK) sendAuthRequest(port, AUTH_REQ_OK, NULL, 0); diff --git a/src/postgres/src/backend/libpq/hba.c b/src/postgres/src/backend/libpq/hba.c index 996b0caedf0a..cff59fd2e831 100644 --- a/src/postgres/src/backend/libpq/hba.c +++ b/src/postgres/src/backend/libpq/hba.c @@ -75,8 +75,6 @@ typedef struct check_network_data static List *parsed_hba_lines = NIL; static MemoryContext parsed_hba_context = NULL; -#ifdef YB_TODO -/* YB_TODO(neil) Need to redo JasonK code. It's not merge-able */ /* * The following character array contains the additional hardcoded HBA config * lines that are set internally. These lines take priority over user defined @@ -86,7 +84,6 @@ static const char *const HardcodedHbaLines[] = { "local all postgres yb-tserver-key", }; -#endif /* * pre-parsed content of ident mapping file: list of IdentLine structs. @@ -128,13 +125,10 @@ static const char *const UserAuthName[] = static List *tokenize_inc_file(List *tokens, const char *outer_filename, const char *inc_filename, int elevel, char **err_msg); -static void tokenize_hardcoded(List **tok_lines_all, int elevel); -#ifdef YB_TODO -/* YB_TODO(neil) Need to change JasonK code. It's not merge-able. */ -static TokenizedLine *tokenize_line(const char *filename, int elevel, - int line_number, char *rawline, - char *err_msg); -#endif +static void yb_tokenize_hardcoded(List **tok_lines_all, int elevel); +static TokenizedAuthLine *yb_tokenize_line(const char *filename, int elevel, + int line_number, char *rawline, + char *err_msg); static bool parse_hba_auth_opt(char *name, char *val, HbaLine *hbaline, int elevel, char **err_msg); @@ -463,16 +457,6 @@ tokenize_inc_file(List *tokens, * Return value is a memory context which contains all memory allocated by * this function (it's a child of caller's context). */ -#ifdef YB_TODO -/* - * YB_TODO(jasonk@yugabyte) - * This is Postgres version for PG13. - * - Please do not factor postgres existing functions. It makes it extremely difficult to merge. - * - Write your own version if you want, but leave postgres version alone. - * - Keep in mind that code-merging people are not supposed to learn your projects line-by-line - * and logically resolve conflicts. - */ -#endif MemoryContext tokenize_auth_file(const char *filename, FILE *file, List **tok_lines, int elevel) @@ -482,7 +466,7 @@ tokenize_auth_file(const char *filename, FILE *file, List **tok_lines, MemoryContext linecxt; MemoryContext oldcxt; - linecxt = AllocSetContextCreate(CurrentMemoryContext, + linecxt = AllocSetContextCreate(GetCurrentMemoryContext(), "tokenize_auth_file", ALLOCSET_SMALL_SIZES); oldcxt = MemoryContextSwitchTo(linecxt); @@ -575,108 +559,20 @@ tokenize_auth_file(const char *filename, FILE *file, List **tok_lines, return linecxt; } -#ifdef YB_TODO -/* - * YB_TODO(jasonk@yugabyte) Need to verify if merging code is correct. - * - Please do not factor postgres existing functions. It makes it extremely difficult to merge. - * - Write your own version if you want, but leave postgres version alone. - * - Keep in mind that code-merging people are not supposed to review and learn your projects or - * entire team's projects line-by-line and logically resolve conflicts. - */ -static MemoryContext -tokenize_file_jasonk_version(const char *filename, FILE *file, List **tok_lines, int elevel) -{ - int line_number = 1; - StringInfoData buf; - MemoryContext linecxt; - MemoryContext oldcxt; - - linecxt = AllocSetContextCreate(GetCurrentMemoryContext(), - "tokenize_file", - ALLOCSET_SMALL_SIZES); - oldcxt = MemoryContextSwitchTo(linecxt); - - initStringInfo(&buf); - - *tok_lines = NIL; - - while (!feof(file) && !ferror(file)) - { - char *err_msg = NULL; - int last_backslash_buflen = 0; - int continuations = 0; - TokenizedLine *tok_line; - - /* Collect the next input line, handling backslash continuations */ - resetStringInfo(&buf); - - while (pg_get_line_append(file, &buf)) - { - /* Strip trailing newline, including \r in case we're on Windows */ - buf.len = pg_strip_crlf(buf.data); - - /* - * Check for backslash continuation. The backslash must be after - * the last place we found a continuation, else two backslashes - * followed by two \n's would behave surprisingly. - */ - if (buf.len > last_backslash_buflen && - buf.data[buf.len - 1] == '\\') - { - /* Continuation, so strip it and keep reading */ - buf.data[--buf.len] = '\0'; - last_backslash_buflen = buf.len; - continuations++; - continue; - } - - /* Nope, so we have the whole line */ - break; - } - - if (ferror(file)) - { - /* I/O error! */ - int save_errno = errno; - - ereport(elevel, - (errcode_for_file_access(), - errmsg("could not read file \"%s\": %m", filename))); - err_msg = psprintf("could not read file \"%s\": %s", - filename, strerror(save_errno)); - break; - } - - tok_line = tokenize_line(filename, - elevel, - line_number, - buf.data, - err_msg); - if (tok_line != NULL) - *tok_lines = lappend(*tok_lines, tok_line); - - line_number++; - } - - MemoryContextSwitchTo(oldcxt); - - return linecxt; -} - /* * Tokenize the hardcoded configuration lines. * - * The output is a list of TokenizedLine structs; see struct definition above. + * The output is a list of TokenizedAuthLine structs; see struct definition above. * * tok_lines: receives output list * elevel: message logging level * * Errors are reported by logging messages at ereport level elevel and by - * adding TokenizedLine structs containing non-null err_msg fields to the + * adding TokenizedAuthLine structs containing non-null err_msg fields to the * output list. */ static void -tokenize_hardcoded(List **tok_lines, int elevel) +yb_tokenize_hardcoded(List **tok_lines, int elevel) { int line_number = -1; @@ -685,13 +581,11 @@ tokenize_hardcoded(List **tok_lines, int elevel) for (int i = 0; i < sizeof(HardcodedHbaLines) / sizeof(char *); ++i) { char *err_msg = NULL; - TokenizedLine *tok_line; + TokenizedAuthLine *tok_line; - tok_line = tokenize_line("(hardcoded: no filename)" /* filename */, - elevel, - line_number, - pstrdup(HardcodedHbaLines[i]), - err_msg); + tok_line = yb_tokenize_line("(hardcoded: no filename)" /* filename */, + elevel, line_number, + pstrdup(HardcodedHbaLines[i]), err_msg); if (tok_line != NULL) *tok_lines = lappend(*tok_lines, tok_line); @@ -702,7 +596,7 @@ tokenize_hardcoded(List **tok_lines, int elevel) /* * Tokenize the given line. * - * The output is a TokenizedLine struct. + * The output is a TokenizedAuthLine struct. * * filename: the absolute path to the target file * elevel: message logging level @@ -711,12 +605,12 @@ tokenize_hardcoded(List **tok_lines, int elevel) * err_msg: error message (inherit it, set it, or leave it null) * * Errors are reported by logging messages at ereport level elevel and by - * putting a non-null err_msg in the TokenizedLine struct. + * putting a non-null err_msg in the TokenizedAuthLine struct. * * Return value is a palloc'd tokenized line. */ -static TokenizedLine * -tokenize_line(const char *filename, +static TokenizedAuthLine * +yb_tokenize_line(const char *filename, int elevel, int line_number, char *rawline, @@ -731,9 +625,7 @@ tokenize_line(const char *filename, errmsg("line is null"))); /* Strip trailing linebreak from rawline */ - lineptr = rawline + strlen(rawline) - 1; - while (lineptr >= rawline && (*lineptr == '\n' || *lineptr == '\r')) - *lineptr-- = '\0'; + pg_strip_crlf(rawline); /* Parse fields */ lineptr = rawline; @@ -751,9 +643,9 @@ tokenize_line(const char *filename, /* Reached EOL; emit line unless it's boring */ if (current_line != NIL || err_msg != NULL) { - TokenizedLine *tok_line; + TokenizedAuthLine *tok_line; - tok_line = (TokenizedLine *) palloc(sizeof(TokenizedLine)); + tok_line = (TokenizedAuthLine *) palloc(sizeof(TokenizedAuthLine)); tok_line->fields = current_line; tok_line->line_num = line_number; tok_line->raw_line = pstrdup(rawline); @@ -763,8 +655,6 @@ tokenize_line(const char *filename, return NULL; } -#endif - /* * Does user belong to role? * @@ -2507,10 +2397,7 @@ load_hba(void) List *hba_lines_hardcoded = NIL; oldcxt = MemoryContextSwitchTo(linecxt); -#ifdef YB_TODO - /* YB_TODO(neil) Need to change JasonK code. It's not merge-able */ - tokenize_hardcoded(&hba_lines_hardcoded, LOG); -#endif + yb_tokenize_hardcoded(&hba_lines_hardcoded, LOG); hba_lines = list_concat(hba_lines_hardcoded, hba_lines); MemoryContextSwitchTo(oldcxt); diff --git a/src/postgres/src/backend/tcop/utility.c b/src/postgres/src/backend/tcop/utility.c index 337dc015e5d4..34510ae3c8b8 100644 --- a/src/postgres/src/backend/tcop/utility.c +++ b/src/postgres/src/backend/tcop/utility.c @@ -413,6 +413,7 @@ ClassifyUtilityCommandAsReadOnly(Node *parsetree) return 0; /* silence stupider compilers */ } + case T_BackfillIndexStmt: case T_CreateTableGroupStmt: case T_YbCreateProfileStmt: case T_YbDropProfileStmt: diff --git a/src/postgres/src/test/regress/expected/yb_pg15.out b/src/postgres/src/test/regress/expected/yb_pg15.out index 9af55910e0d7..9eac8fecd661 100644 --- a/src/postgres/src/test/regress/expected/yb_pg15.out +++ b/src/postgres/src/test/regress/expected/yb_pg15.out @@ -117,6 +117,8 @@ select * from t2; 1 | Jane | 1 (1 row) +-- CREATE INDEX +CREATE INDEX myidx on t2(name); -- Insert with on conflict insert into t2 values (1, 'foo') on conflict ON CONSTRAINT t2_pkey do update set id = t2.id+1; select * from t2; diff --git a/src/postgres/src/test/regress/sql/yb_pg15.sql b/src/postgres/src/test/regress/sql/yb_pg15.sql index 79eef78182d9..18379b463c41 100644 --- a/src/postgres/src/test/regress/sql/yb_pg15.sql +++ b/src/postgres/src/test/regress/sql/yb_pg15.sql @@ -56,6 +56,9 @@ update t2 set name = 'Jane' where id = 1; select * from t2; +-- CREATE INDEX +CREATE INDEX myidx on t2(name); + -- Insert with on conflict insert into t2 values (1, 'foo') on conflict ON CONSTRAINT t2_pkey do update set id = t2.id+1;