From e0774d605680902d778cb6de8c557a9db2856409 Mon Sep 17 00:00:00 2001 From: Deepakshi Mittal Date: Sun, 30 Jul 2023 18:07:59 +0000 Subject: [PATCH] Code review comments Corrected formatting Added case for decimal used inbuilt function for getting procedure name Added syntax error if identity_into function is called directly Task: BABEL-539 Signed-off-by: Deepakshi Mittal --- contrib/babelfishpg_tsql/runtime/functions.c | 2 +- .../babelfishpg_tsql/sql/sys_functions.sql | 1 + .../babelfishpg_tsql--3.2.0--3.3.0.sql | 1 + .../src/backend_parser/gram-tsql-epilogue.y.c | 5 +- contrib/babelfishpg_tsql/src/pl_handler.c | 182 +++++++++--------- contrib/babelfishpg_tsql/src/tsqlIface.cpp | 26 +-- test/JDBC/expected/BABEL_539-vu-verify.out | 30 ++- test/JDBC/input/BABEL_539-vu-verify.sql | 12 +- 8 files changed, 156 insertions(+), 103 deletions(-) diff --git a/contrib/babelfishpg_tsql/runtime/functions.c b/contrib/babelfishpg_tsql/runtime/functions.c index ba9e653299..a5c91f242f 100644 --- a/contrib/babelfishpg_tsql/runtime/functions.c +++ b/contrib/babelfishpg_tsql/runtime/functions.c @@ -1905,7 +1905,7 @@ identity_into(PG_FUNCTION_ARGS) int64 result; Assert(tsql_select_into_seq_oid != InvalidOid); result = nextval_internal(tsql_select_into_seq_oid, false); - return result; + PG_RETURN_INT64((int64) result); } /* diff --git a/contrib/babelfishpg_tsql/sql/sys_functions.sql b/contrib/babelfishpg_tsql/sql/sys_functions.sql index ddc9986235..e6f942a271 100644 --- a/contrib/babelfishpg_tsql/sql/sys_functions.sql +++ b/contrib/babelfishpg_tsql/sql/sys_functions.sql @@ -3393,6 +3393,7 @@ GRANT EXECUTE ON FUNCTION sys.host_id() TO PUBLIC; CREATE OR REPLACE FUNCTION sys.identity_into(IN typename INT, IN seed INT, IN increment INT) RETURNS int AS 'babelfishpg_tsql' LANGUAGE C STABLE; +GRANT EXECUTE ON FUNCTION sys.identity_into(INT, INT, INT) TO PUBLIC; CREATE OR REPLACE FUNCTION sys.degrees(IN arg1 BIGINT) RETURNS bigint AS 'babelfishpg_tsql','bigint_degrees' LANGUAGE C STRICT IMMUTABLE PARALLEL SAFE; diff --git a/contrib/babelfishpg_tsql/sql/upgrades/babelfishpg_tsql--3.2.0--3.3.0.sql b/contrib/babelfishpg_tsql/sql/upgrades/babelfishpg_tsql--3.2.0--3.3.0.sql index b4ddfef7fd..e841005455 100644 --- a/contrib/babelfishpg_tsql/sql/upgrades/babelfishpg_tsql--3.2.0--3.3.0.sql +++ b/contrib/babelfishpg_tsql/sql/upgrades/babelfishpg_tsql--3.2.0--3.3.0.sql @@ -44,6 +44,7 @@ LANGUAGE C IMMUTABLE STRICT; CREATE OR REPLACE FUNCTION sys.identity_into(IN typename INT, IN seed INT, IN increment INT) RETURNS int AS 'babelfishpg_tsql' LANGUAGE C STABLE; +GRANT EXECUTE ON FUNCTION sys.identity_into(INT, INT, INT) TO PUBLIC; CREATE OR REPLACE VIEW sys.sql_expression_dependencies AS diff --git a/contrib/babelfishpg_tsql/src/backend_parser/gram-tsql-epilogue.y.c b/contrib/babelfishpg_tsql/src/backend_parser/gram-tsql-epilogue.y.c index 5464a78c98..fc870925db 100644 --- a/contrib/babelfishpg_tsql/src/backend_parser/gram-tsql-epilogue.y.c +++ b/contrib/babelfishpg_tsql/src/backend_parser/gram-tsql-epilogue.y.c @@ -246,13 +246,14 @@ TsqlFunctionIdentityInto(TypeName *typename, Node *seed, Node *increment, int lo case INT4OID: case INT8OID: case NUMERICOID: - args = list_make3((Node *)makeIntConst((int)type_oid, -1), seed, increment); + args = list_make3((Node *)makeIntConst((int)type_oid, location), seed, increment); result = (Node *) makeFuncCall(TsqlSystemFuncName("identity_into"), args, COERCE_EXPLICIT_CALL, location); break; default: ereport(ERROR, (errcode(ERRCODE_SYNTAX_ERROR), - errmsg("identity column type must be smallint, integer, or bigint"))); + errmsg("identity column type must be smallint, integer, bigint, or numeric"))); + break; } diff --git a/contrib/babelfishpg_tsql/src/pl_handler.c b/contrib/babelfishpg_tsql/src/pl_handler.c index 34392ccd6e..b5cd391d10 100644 --- a/contrib/babelfishpg_tsql/src/pl_handler.c +++ b/contrib/babelfishpg_tsql/src/pl_handler.c @@ -142,7 +142,7 @@ Datum sp_prepare(PG_FUNCTION_ARGS); Datum sp_unprepare(PG_FUNCTION_ARGS); static List *transformReturningList(ParseState *pstate, List *returningList); static List *transformSelectIntoStmt(CreateTableAsStmt *stmt, const char *queryString); -static char *parse_type_argument(int type_oid); +static char *get_oid_type_string(int type_oid); extern char *construct_unique_index_name(char *index_name, char *relation_name); extern int CurrentLineNumber; static non_tsql_proc_entry_hook_type prev_non_tsql_proc_entry_hook = NULL; @@ -5026,9 +5026,16 @@ pltsql_revert_guc(int nest_level) } -static char *parse_type_argument(int type_oid){ +static char *get_oid_type_string(int type_oid){ char *type_string = NULL; - switch(type_oid){ + if ((*common_utility_plugin_ptr->is_tsql_decimal_datatype) (type_oid)) + { + type_string = "decimal"; + return type_string; + } + + switch(type_oid) + { case INT2OID: type_string = "pg_catalog.int2"; break; @@ -5042,18 +5049,20 @@ static char *parse_type_argument(int type_oid){ type_string = "pg_catalog.numeric"; break; default: - type_string = "decimal"; - break; + ereport(ERROR, (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), + errmsg("Identity column type must be smallint, integer, bigint, or numeric"))); + break; } return type_string; } -static List * transformSelectIntoStmt(CreateTableAsStmt *stmt, const char *queryString){ - List *result; - ListCell *elements; +static List *transformSelectIntoStmt(CreateTableAsStmt *stmt, const char *queryString) +{ + List *result; + ListCell *elements; CreateSeqStmt *seqstmt; AlterSeqStmt *altseqstmt; - List *attnamelist; + List *attnamelist; IntoClause *into; Node *n; @@ -5065,62 +5074,69 @@ static List * transformSelectIntoStmt(CreateTableAsStmt *stmt, const char *query if (n && n->type == T_Query) { - Query *q = (Query *) n; + Query *q = (Query *)n; bool seen_identity = false; - foreach(elements, q->targetList) - { - TargetEntry *tle = (TargetEntry *) lfirst(elements); - FuncExpr *funcexpr; - if (tle->expr && IsA(tle->expr, FuncExpr) && strcasecmp(get_func_name(((FuncExpr *) (tle->expr))->funcid), "identity_into") ==0 ){ - - Oid snamespaceid; + foreach (elements, q->targetList) + { + TargetEntry *tle = (TargetEntry *)lfirst(elements); + + if (tle->expr && IsA(tle->expr, FuncExpr) && strcasecmp(get_func_name(((FuncExpr *)(tle->expr))->funcid), "identity_into") == 0) + { + FuncExpr *funcexpr; + Oid snamespaceid; char *snamespace; char *sname; List *seqoptions = NIL; ListCell *arg; int type_oid; - char *type= NULL; + char *type = NULL; TypeName *ofTypename; int64 seed_value; int arg_num; - if(seen_identity){ + if (seen_identity) + { ereport(ERROR, - (errcode(ERRCODE_SYNTAX_ERROR), errmsg("Attempting to add multiple identity columns to table \"%s\" using the SELECT INTO statement.", into->rel->relname ))); + (errcode(ERRCODE_SYNTAX_ERROR), errmsg("Attempting to add multiple identity columns to table \"%s\" using the SELECT INTO statement.", into->rel->relname))); } - if(tle->resname == NULL){ + if (tle->resname == NULL) + { ereport(ERROR, - (errcode(ERRCODE_SYNTAX_ERROR), errmsg("Incorrect syntax near the keyword 'INTO'"))); + (errcode(ERRCODE_SYNTAX_ERROR), errmsg("Incorrect syntax near the keyword 'INTO'"))); } funcexpr = (FuncExpr *)tle->expr; arg_num = 0; - foreach(arg, funcexpr->args) + foreach (arg, funcexpr->args) { - Node *fargNode = (Node *) lfirst(arg); + Node *fargNode = (Node *)lfirst(arg); Const *con; arg_num++; - switch (arg_num){ + switch (arg_num) + { case 1: - con = (Const *) fargNode; - type_oid = (int) con->constvalue; - type = parse_type_argument(type_oid); + con = (Const *)fargNode; + type_oid = (int)con->constvalue; + type = get_oid_type_string(type_oid); ofTypename = typeStringToTypeName(type); - seqoptions = lappend(seqoptions, makeDefElem("as", (Node *) ofTypename, -1)); + seqoptions = lappend(seqoptions, makeDefElem("as", (Node *)ofTypename, -1)); break; case 2: - con = (Const *) fargNode; - seqoptions = lappend(seqoptions, makeDefElem("start", (Node *)makeInteger(con->constvalue), -1)); - seed_value = (int64) con->constvalue; + con = (Const *)fargNode; + seqoptions = lappend(seqoptions, makeDefElem("start", (Node *)makeInteger((int64)con->constvalue), -1)); + seed_value = (int64)con->constvalue; break; case 3: - con = (Const *) fargNode; - seqoptions = lappend(seqoptions, makeDefElem("increment", (Node *)makeInteger(con->constvalue), -1)); - if ((int) con->constvalue > 0){ + con = (Const *)fargNode; + seqoptions = lappend(seqoptions, makeDefElem("increment", (Node *)makeInteger((int64)con->constvalue), -1)); + if ((int)con->constvalue > 0) + { seqoptions = lappend(seqoptions, makeDefElem("minvalue", (Node *)makeInteger(seed_value), -1)); - }else{ + } + else + { seqoptions = lappend(seqoptions, makeDefElem("maxvalue", (Node *)makeInteger(seed_value), -1)); } break; @@ -5134,7 +5150,7 @@ static List * transformSelectIntoStmt(CreateTableAsStmt *stmt, const char *query snamespaceid = RangeVarGetCreationNamespace(into->rel); snamespace = get_namespace_name(snamespaceid); - sname = ChooseRelationName(into->rel->relname, tle->resname,"seq",snamespaceid, false); + sname = ChooseRelationName(into->rel->relname, tle->resname, "seq", snamespaceid, false); seqstmt = makeNode(CreateSeqStmt); seqstmt->for_identity = true; seqstmt->sequence = makeRangeVar(snamespace, sname, -1); @@ -5143,74 +5159,68 @@ static List * transformSelectIntoStmt(CreateTableAsStmt *stmt, const char *query altseqstmt = makeNode(AlterSeqStmt); altseqstmt->sequence = makeRangeVar(snamespace, sname, -1); - attnamelist = list_make3(makeString(snamespace), makeString(into->rel->relname),makeString(tle->resname)); - altseqstmt->options = list_make1(makeDefElem("owned_by", (Node *) attnamelist, -1)); + attnamelist = list_make3(makeString(snamespace), makeString(into->rel->relname), makeString(tle->resname)); + altseqstmt->options = list_make1(makeDefElem("owned_by", (Node *)attnamelist, -1)); altseqstmt->for_identity = true; - } } } - if(seqstmt){ - result = lappend(result, seqstmt); + if (seqstmt) + { + result = lappend(result, seqstmt); } result = lappend(result, stmt); - - if(altseqstmt){ - result = lappend(result, altseqstmt); + if (altseqstmt) + { + result = lappend(result, altseqstmt); } - return result; + return result; } +void pltsql_bbfSelectIntoUtility(ParseState *pstate, PlannedStmt *pstmt, const char *queryString, QueryEnvironment *queryEnv, + ParamListInfo params, QueryCompletion *qc) +{ -void -pltsql_bbfSelectIntoUtility(ParseState *pstate, PlannedStmt *pstmt, const char *queryString, QueryEnvironment *queryEnv, - ParamListInfo params, QueryCompletion *qc){ - - Node *parsetree = pstmt->utilityStmt; + Node *parsetree = pstmt->utilityStmt; ObjectAddress address; ObjectAddress secondaryObject = InvalidObjectAddress; List *stmts; - stmts = transformSelectIntoStmt((CreateTableAsStmt *) parsetree, queryString); - while (stmts != NIL) + stmts = transformSelectIntoStmt((CreateTableAsStmt *)parsetree, queryString); + while (stmts != NIL) + { + Node *stmt = (Node *)linitial(stmts); + stmts = list_delete_first(stmts); + if (IsA(stmt, CreateTableAsStmt)) { - Node *stmt = (Node *) linitial(stmts); - stmts = list_delete_first(stmts); - if (IsA(stmt, CreateTableAsStmt)) - { - address = ExecCreateTableAs(pstate, (CreateTableAsStmt *) parsetree,params, queryEnv, qc); - EventTriggerCollectSimpleCommand(address,secondaryObject,stmt); - } - else if(IsA(stmt, CreateSeqStmt)) { - address = DefineSequence(pstate, (CreateSeqStmt *) stmt); - Assert(address.objectId != InvalidOid); - tsql_select_into_seq_oid = address.objectId; - EventTriggerCollectSimpleCommand(address,secondaryObject,stmt); - } - else{ - - PlannedStmt *wrapper; - wrapper = makeNode(PlannedStmt); - wrapper->commandType = CMD_UTILITY; - wrapper->canSetTag = false; - wrapper->utilityStmt = stmt; - wrapper->stmt_location = pstmt->stmt_location; - wrapper->stmt_len = pstmt->stmt_len; - - ProcessUtility(wrapper, - queryString, - false, - PROCESS_UTILITY_SUBCOMMAND, - params, - NULL, - None_Receiver, - NULL); - } + address = ExecCreateTableAs(pstate, (CreateTableAsStmt *)parsetree, params, queryEnv, qc); + EventTriggerCollectSimpleCommand(address, secondaryObject, stmt); + } + else if (IsA(stmt, CreateSeqStmt)) + { + address = DefineSequence(pstate, (CreateSeqStmt *)stmt); + Assert(address.objectId != InvalidOid); + tsql_select_into_seq_oid = address.objectId; + EventTriggerCollectSimpleCommand(address, secondaryObject, stmt); + } + else + { + PlannedStmt *wrapper; + wrapper = makeNode(PlannedStmt); + wrapper->commandType = CMD_UTILITY; + wrapper->canSetTag = false; + wrapper->utilityStmt = stmt; + wrapper->stmt_location = pstmt->stmt_location; + wrapper->stmt_len = pstmt->stmt_len; + + ProcessUtility(wrapper, queryString, false, PROCESS_UTILITY_SUBCOMMAND, params, NULL, None_Receiver, NULL); + } if (stmts != NIL) CommandCounterIncrement(); - } + } } + void set_current_query_is_create_tbl_check_constraint(Node *expr) { diff --git a/contrib/babelfishpg_tsql/src/tsqlIface.cpp b/contrib/babelfishpg_tsql/src/tsqlIface.cpp index 0d2bfb1c39..219c609750 100644 --- a/contrib/babelfishpg_tsql/src/tsqlIface.cpp +++ b/contrib/babelfishpg_tsql/src/tsqlIface.cpp @@ -1847,17 +1847,6 @@ class tsqlBuilder : public tsqlCommonMutator } } - void enterFunction_call(TSqlParser::Function_callContext *ctx) override - { - std::string func_name = ::getFullText(ctx); - size_t Lbracket_index = func_name.find('('); - func_name = func_name.substr(0, Lbracket_index); - if (pg_strcasecmp(func_name.c_str(), "identity") == 0 || pg_strcasecmp(func_name.c_str(), "identity_into") == 0 - || pg_strcasecmp(func_name.c_str(), "sys.identity_into") == 0) { - has_identity_function = true; - } - } - ////////////////////////////////////////////////////////////////////////////// // function/procedure call analysis ////////////////////////////////////////////////////////////////////////////// @@ -1966,6 +1955,21 @@ class tsqlBuilder : public tsqlCommonMutator } } + + if (ctx->func_proc_name_server_database_schema()->procedure) + { + std::string proc_name = stripQuoteFromId(ctx->func_proc_name_server_database_schema()->procedure); + if (pg_strcasecmp(proc_name.c_str(), "identity") == 0) + { + has_identity_function = true; + } + + if (pg_strcasecmp(proc_name.c_str(), "identity_into") == 0) + { + throw PGErrorWrapperException(ERROR, ERRCODE_FEATURE_NOT_SUPPORTED, + format_errmsg("function %s does not exist", proc_name.c_str()), getLineAndPos(ctx)); + } + } } } diff --git a/test/JDBC/expected/BABEL_539-vu-verify.out b/test/JDBC/expected/BABEL_539-vu-verify.out index 5913a18679..fc728d1a45 100644 --- a/test/JDBC/expected/BABEL_539-vu-verify.out +++ b/test/JDBC/expected/BABEL_539-vu-verify.out @@ -123,7 +123,7 @@ SELECT col1, IDENTITY(char, 1,1) AS id_num INTO babel_539NewTable1 FROM babel_53 GO ~~ERROR (Code: 33557097)~~ -~~ERROR (Message: identity column type must be smallint, integer, or bigint)~~ +~~ERROR (Message: identity column type must be smallint, integer, bigint, or numeric)~~ DROP TABLE IF EXISTS babel_539NewTable1; @@ -208,5 +208,31 @@ GO ~~ERROR (Message: Attempting to add multiple identity columns to table "#babel_539newtemptable2" using the SELECT INTO statement.)~~ -DROP TABLE IF EXISTS #babel_539NewTempTable2; +--calling internal function directly +SELECT col1, IDENTITY_INTO(1, 1,1) as id_num INTO #babel_539NewTempTable2 FROM babel_539OldTable; +GO +~~ERROR (Code: 33557097)~~ + +~~ERROR (Message: function IDENTITY_INTO does not exist)~~ + + +SELECT sys.IDENTITY(23, 1); GO +~~ERROR (Code: 33557097)~~ + +~~ERROR (Message: Incorrect syntax near ')')~~ + + +SELECT IDENTITY(int, 21); +GO +~~ERROR (Code: 33557097)~~ + +~~ERROR (Message: Incorrect syntax near ')')~~ + + +SELECT sys.IDENTITY_INTO(23, 1, 1); +GO +~~ERROR (Code: 33557097)~~ + +~~ERROR (Message: function IDENTITY_INTO does not exist)~~ + diff --git a/test/JDBC/input/BABEL_539-vu-verify.sql b/test/JDBC/input/BABEL_539-vu-verify.sql index d2b0be9402..c6154da9d1 100644 --- a/test/JDBC/input/BABEL_539-vu-verify.sql +++ b/test/JDBC/input/BABEL_539-vu-verify.sql @@ -131,5 +131,15 @@ GO SELECT col1, IDENTITY(int, 1,1) as id_num, IDENTITY(int, 1,1) as id_num2 INTO #babel_539NewTempTable2 FROM babel_539OldTable; GO -DROP TABLE IF EXISTS #babel_539NewTempTable2; +--calling internal function directly +SELECT col1, IDENTITY_INTO(1, 1,1) as id_num INTO #babel_539NewTempTable2 FROM babel_539OldTable; +GO + +SELECT sys.IDENTITY(23, 1); +GO + +SELECT IDENTITY(int, 21); +GO + +SELECT sys.IDENTITY_INTO(23, 1, 1); GO \ No newline at end of file