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

insert varchar column mismatch through INSERT INTO EXEC into a table #2017

Merged
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
234 changes: 229 additions & 5 deletions contrib/babelfishpg_tsql/src/pl_exec-2.c
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,9 @@
#include "funcapi.h"

#include "access/table.h"
#include "access/attmap.h"
#include "catalog/namespace.h"
#include "catalog/pg_attribute.h"
#include "catalog/pg_language.h"
#include "commands/proclang.h"
#include "executor/tstoreReceiver.h"
Expand All @@ -24,6 +26,17 @@
PLtsql_execstate *get_current_tsql_estate(void);
PLtsql_execstate *get_outermost_tsql_estate(int *nestlevel);

typedef struct TupleCastMap
forestkeeper marked this conversation as resolved.
Show resolved Hide resolved
{
TupleDesc indesc; /* tupdesc for source rowtype */
TupleDesc outdesc; /* tupdesc for result rowtype */
AttrMap *attrMap; /* indexes of input fields, or 0 for null */
Datum *invalues; /* workspace for deconstructing source */
bool *inisnull;
Datum *outvalues; /* workspace for constructing result */
bool *outisnull;
} TupleCastMap;

/*
* NOTE:
* A SET...(SELECT) statement that returns more than one row will raise an error
Expand Down Expand Up @@ -113,6 +126,15 @@ static bool prev_insert_bulk_keep_nulls = false;
/* return a underlying node if n is implicit casting and underlying node is a certain type of node */
static Node *get_underlying_node_from_implicit_casting(Node *n, NodeTag underlying_nodetype);

static TupleCastMap *cast_tuples_by_position(TupleDesc indesc,
TupleDesc outdesc,
const char *msg);
static AttrMap* build_typecast_attrmap_by_position(TupleDesc indesc, TupleDesc outdesc, const char *msg);
static HeapTuple exec_cast_tuple(HeapTuple tuple, TupleCastMap *tupleCastMap);
static bool
check_attrmap_match(TupleDesc indesc,
TupleDesc outdesc,
AttrMap *attrMap);
/*
* The pltsql_proc_return_code global variable is used to record the
* return code (RETURN 41 + 1) of the most recently completed procedure
Expand Down Expand Up @@ -2995,7 +3017,7 @@ exec_stmt_insert_execute_select(PLtsql_execstate *estate, PLtsql_expr *query)
{
Portal portal;
uint64 processed = 0;
TupleConversionMap *tupmap;
TupleCastMap *tupmap;
MemoryContext oldcontext;

if (estate->tuple_store == NULL)
Expand All @@ -3007,7 +3029,7 @@ exec_stmt_insert_execute_select(PLtsql_execstate *estate, PLtsql_expr *query)
/* Use eval_mcontext for tuple conversion work */
oldcontext = MemoryContextSwitchTo(get_eval_mcontext(estate));

tupmap = convert_tuples_by_position(portal->tupDesc,
tupmap = cast_tuples_by_position(portal->tupDesc,
estate->tuple_store_desc,
gettext_noop("structure of query does not match function result type"));

Expand All @@ -3028,7 +3050,7 @@ exec_stmt_insert_execute_select(PLtsql_execstate *estate, PLtsql_expr *query)
HeapTuple tuple = SPI_tuptable->vals[i];

if (tupmap)
tuple = execute_attr_map_tuple(tuple, tupmap);
tuple = exec_cast_tuple(tuple, tupmap);
tuplestore_puttuple(estate->tuple_store, tuple);
if (tupmap)
heap_freetuple(tuple);
Expand All @@ -3047,6 +3069,210 @@ exec_stmt_insert_execute_select(PLtsql_execstate *estate, PLtsql_expr *query)
return PLTSQL_RC_OK;
}

static AttrMap* build_typecast_attrmap_by_position(TupleDesc indesc, TupleDesc outdesc, const char *msg)
Copy link
Contributor

@2jungkook 2jungkook Nov 14, 2023

Choose a reason for hiding this comment

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

It seems like this is very similar to build_attrmap_by_position(). In that case, adding a special handling to the function can be better, if possible (either by adding a new hook or dialect check). If we copy engine's function, it will be not easy to adopt a new major versions when there are some changes in the original function.

{
AttrMap *attrMap;
int nincols;
int noutcols;
int n;
int i;
int j;
bool same;

/*
* The length is computed as the number of attributes of the expected
* rowtype as it includes dropped attributes in its count.
*/
n = outdesc->natts;
attrMap = make_attrmap(n);

j = 0; /* j is next physical input attribute */
nincols = noutcols = 0; /* these count non-dropped attributes */
same = true;
for (i = 0; i < n; i++)
{
Form_pg_attribute outatt = TupleDescAttr(outdesc, i);

if (outatt->attisdropped)
continue; /* attrMap->attnums[i] is already 0 */
noutcols++;
for (; j < indesc->natts; j++)
{
Form_pg_attribute inatt = TupleDescAttr(outdesc, j);
if (inatt->attisdropped)
continue;
nincols++;

attrMap->attnums[i] = (AttrNumber) (j + 1);
j++;
break;
}
if (attrMap->attnums[i] == 0)
same = false; /* we'll complain below */
}

/* Check for unused input columns */
for (; j < indesc->natts; j++)
{
if (TupleDescAttr(indesc, j)->attisdropped)
continue;
nincols++;
same = false; /* we'll complain below */
}

/* Report column count mismatch using the non-dropped-column counts */
if (!same)
ereport(ERROR,
(errcode(ERRCODE_DATATYPE_MISMATCH),
errmsg_internal("%s", _(msg)),
errdetail("Number of returned columns (%d) does not match "
"expected column count (%d).",
nincols, noutcols)));

/* Check if the map has a one-to-one match */
if (check_attrmap_match(indesc, outdesc, attrMap))
{
/* Runtime conversion is not needed */
free_attrmap(attrMap);
return NULL;
}

return attrMap;
}

/*
* check_attrmap_match
*
* Check to see if the map is a one-to-one match, in which case we need
* not to do a tuple conversion, and the attribute map is not necessary.
*/
static bool
check_attrmap_match(TupleDesc indesc,
TupleDesc outdesc,
AttrMap *attrMap)
{
int i;

/* no match if attribute numbers are not the same */
if (indesc->natts != outdesc->natts)
return false;

for (i = 0; i < attrMap->maplen; i++)
{
Form_pg_attribute inatt = TupleDescAttr(indesc, i);
Form_pg_attribute outatt = TupleDescAttr(outdesc, i);

/*
* If the input column has a missing attribute, we need a conversion.
*/
if (inatt->atthasmissing)
return false;

if (inatt->atttypid != outatt->atttypid ||
inatt->atttypmod != outatt->atttypmod)
return false;

if (attrMap->attnums[i] == (i + 1))
continue;

/*
* If it's a dropped column and the corresponding input column is also
* dropped, we don't need a conversion. However, attlen and attalign
* must agree.
*/
if (attrMap->attnums[i] == 0 &&
inatt->attisdropped &&
inatt->attlen == outatt->attlen &&
inatt->attalign == outatt->attalign)
continue;

return false;
}

return true;
}

static TupleCastMap *cast_tuples_by_position(TupleDesc indesc,
TupleDesc outdesc,
const char *msg)
{
TupleCastMap *map;
int n;
AttrMap *attrMap;

/* Verify compatibility and prepare attribute-number map */
attrMap = build_typecast_attrmap_by_position(indesc, outdesc, msg);

if (attrMap == NULL)
{
/* runtime cast is not needed */
return NULL;
}

/* Prepare the map structure */
map = (TupleCastMap *) palloc(sizeof(TupleCastMap));
map->indesc = indesc;
map->outdesc = outdesc;
map->attrMap = attrMap;
/* preallocate workspace for Datum arrays */
n = outdesc->natts + 1; /* +1 for NULL */
map->outvalues = (Datum *) palloc(n * sizeof(Datum));
map->outisnull = (bool *) palloc(n * sizeof(bool));
n = indesc->natts + 1; /* +1 for NULL */
map->invalues = (Datum *) palloc(n * sizeof(Datum));
map->inisnull = (bool *) palloc(n * sizeof(bool));
map->invalues[0] = (Datum) 0; /* set up the NULL entry */
map->inisnull[0] = true;

return map;
}

static HeapTuple exec_cast_tuple(HeapTuple tuple, TupleCastMap *tupleCastMap)
{
AttrMap *attrMap = tupleCastMap->attrMap;
Oid intypeid;
Oid outtypeid;
int inttypemod;
int outtypemod;
Datum *invalues = tupleCastMap->invalues;
bool *inisnull = tupleCastMap->inisnull;
Datum *outvalues = tupleCastMap->outvalues;
bool *outisnull = tupleCastMap->outisnull;
int i;

/*
* Extract all the values of the old tuple, offsetting the arrays so that
* invalues[0] is left NULL and invalues[1] is the first source attribute;
* this exactly matches the numbering convention in attrMap.
*/
heap_deform_tuple(tuple, tupleCastMap->indesc, invalues + 1, inisnull + 1);

/*
* Transpose into proper fields of the new tuple.
*/
Assert(attrMap->maplen == tupleCastMap->outdesc->natts);
for (i = 0; i < attrMap->maplen; i++)
{
int j = attrMap->attnums[i];
Form_pg_attribute outatt = TupleDescAttr(tupleCastMap->outdesc, i);
Form_pg_attribute inatt = TupleDescAttr(tupleCastMap->indesc, i);

outtypeid = outatt->atttypid;
outtypemod = outatt->atttypmod;
intypeid = inatt->atttypid;
inttypemod = inatt->atttypmod;

outisnull[i] = inisnull[j];
outvalues[i] = exec_cast_value(get_current_tsql_estate(),
forestkeeper marked this conversation as resolved.
Show resolved Hide resolved
invalues[j], &outisnull[i],
intypeid, inttypemod,
outtypeid, outtypemod
);
}
// now form the new tuple
return heap_form_tuple(tupleCastMap->outdesc, outvalues, outisnull);
}

int
exec_stmt_insert_bulk(PLtsql_execstate *estate, PLtsql_stmt_insert_bulk *stmt)
{
Expand Down Expand Up @@ -3118,7 +3344,6 @@ exec_stmt_insert_bulk(PLtsql_execstate *estate, PLtsql_stmt_insert_bulk *stmt)
return PLTSQL_RC_OK;
}


int exec_stmt_dbcc(PLtsql_execstate *estate, PLtsql_stmt_dbcc *stmt)
{
switch (stmt->dbcc_stmt_type)
Expand All @@ -3132,7 +3357,6 @@ int exec_stmt_dbcc(PLtsql_execstate *estate, PLtsql_stmt_dbcc *stmt)
return PLTSQL_RC_OK;
}


void exec_stmt_dbcc_checkident(PLtsql_stmt_dbcc *stmt)
{
struct dbcc_checkident dbcc_stmt = stmt->dbcc_stmt_data.dbcc_checkident;
Expand Down
1 change: 0 additions & 1 deletion contrib/babelfishpg_tsql/src/pl_exec.c
Original file line number Diff line number Diff line change
Expand Up @@ -4347,7 +4347,6 @@ pltsql_estate_setup(PLtsql_execstate *estate,
estate->insert_exec = (func->fn_prokind == PROKIND_PROCEDURE ||
strcmp(func->fn_signature, "inline_code_block") == 0)
&& rsi;

estate->pivot_number = 0;
estate->pivot_parsetree_list = NIL;

Expand Down
2 changes: 2 additions & 0 deletions contrib/babelfishpg_tsql/src/pl_handler.c
Original file line number Diff line number Diff line change
Expand Up @@ -4783,6 +4783,7 @@ pltsql_inline_handler(PG_FUNCTION_ARGS)
rsinfo.isDone = ExprSingleResult;
rsinfo.setResult = NULL;
rsinfo.setDesc = NULL;
ReleaseTupleDesc(reldesc);
}

/* And run the function */
Expand Down Expand Up @@ -4849,6 +4850,7 @@ pltsql_inline_handler(PG_FUNCTION_ARGS)
dest->receiveSlot(slot, dest);
ExecClearTuple(slot);
}
ReleaseTupleDesc(rsinfo.expectedDesc);
ExecDropSingleTupleTableSlot(slot);
}

Expand Down
29 changes: 29 additions & 0 deletions test/JDBC/expected/BABEL-2999-vu-cleanup.out
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
drop table if exists t2_BABEL2999;
GO

drop table t1_BABEL2999;
GO

drop table t3_BABEL2999;
GO

drop table t3_BABEL2999_2;
GO

drop procedure p1_BABEL2999;
GO

drop procedure p2_BABEL2999
GO

drop procedure p3_BABEL2999
GO

drop table t4_BABEL2999;
GO

drop table t5_BABEL2999;
GO

drop table t6_BABEL2999
GO
32 changes: 32 additions & 0 deletions test/JDBC/expected/BABEL-2999-vu-prepare.out
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
drop table if exists t1_BABEL2999;
GO

create table t1_BABEL2999(b varchar(10));
GO

create table t2_BABEL2999(b int);
GO

create table t3_BABEL2999(b varchar);
GO

create procedure p1_BABEL2999 as select 'abc';
GO

create procedure p2_BABEL2999 as select 555;
GO

create table t3_BABEL2999_2(a int, b datetime, c varchar(20))
GO

create procedure p3_BABEL2999 as select '123', 123, 123;
GO

create table t4_BABEL2999( a binary(30), b varbinary(30), c varchar(30), d datetime, e smalldatetime)
GO

create table t5_BABEL2999( a decimal, b numeric)
GO

create table t6_BABEL2999( a int, b tinyint, c smallint)
GO
Loading
Loading