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.

Sign up for GitHub

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

[CBRD-24867] rev: regression bug-fix related to DBLINK - CCI_U_TYPE, no serve name #4507

Merged
merged 19 commits into from
Aug 4, 2023
Merged

[CBRD-24867] rev: regression bug-fix related to DBLINK - CCI_U_TYPE, no serve name #4507

Show file tree
Hide file tree
Changes from 17 commits
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
e64de89
[CBRD-24867] rev: regression bug-fix related to DBLINK - CCI_U_TYPE, …
beyondykk9 Jul 24, 2023
7092e90
add m_attr_star, is_select_star, set_select_star
beyondykk9 Jul 24, 2023
8243941
fix: remake select list
beyondykk9 Jul 25, 2023
22cb8d9
add PT_NAME_INFO_DBLINK_SPECIFIED
beyondykk9 Jul 25, 2023
a789da4
add PT_PRINT_SUPPRESS_FOR_DBLINK and check col_cnt 0
beyondykk9 Jul 25, 2023
17e5c75
revert flag PT_NAME_INFO_DBLINK_SPECIFIED
beyondykk9 Jul 25, 2023
c5e9139
add custom print PT_PRINT_NO_HOST_VAR_INDEX
beyondykk9 Jul 26, 2023
061dd9b
reserved word quoted
beyondykk9 Jul 26, 2023
e71a04d
double quoted
beyondykk9 Jul 27, 2023
b4901f7
quoted name and user-name
beyondykk9 Jul 31, 2023
3082a42
fix: generic function
beyondykk9 Jul 31, 2023
0b6f59b
Merge remote-tracking branch 'upstream/develop' into CBRD-24867-buf-f…
beyondykk9 Jul 31, 2023
68666ba
fix: error handling
beyondykk9 Aug 1, 2023
98339d1
modify comment and indent
beyondykk9 Aug 1, 2023
1e26aab
revert to constant folding expr
beyondykk9 Aug 1, 2023
61b8c65
revert to name FLAG for other DBMS
beyondykk9 Aug 2, 2023
1d947c5
name flag revert to short
beyondykk9 Aug 3, 2023
a06ac4c
add error_exit case
beyondykk9 Aug 4, 2023
73bbb2c
mis-spelled comment
beyondykk9 Aug 4, 2023
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
209 changes: 117 additions & 92 deletions src/parser/name_resolution.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -275,6 +275,7 @@ struct remote_tbl_cols

int m_attr_alloc;
int m_attr_used;
bool m_attr_star;
S_REMOTE_COL_ATTR *m_attr;

private:
Expand Down Expand Up @@ -324,6 +325,7 @@ struct remote_tbl_cols
m_nm_buf = NULL;
m_attr_alloc = m_attr_used = 0;
m_attr = NULL;
m_attr_star = false;
}

~remote_tbl_cols ()
Expand Down Expand Up @@ -381,6 +383,16 @@ struct remote_tbl_cols
{
return m_attr_used;
}

bool is_select_star ()
{
return m_attr_star;
}

void set_select_star ()
{
m_attr_star = true;
}
}; /* struct remote_tbl_cols */

static int pt_remake_dblink_select_list (PARSER_CONTEXT * parser, PT_SPEC_INFO * class_spec,
Expand Down Expand Up @@ -1174,6 +1186,9 @@ pt_bind_scope (PARSER_CONTEXT * parser, PT_BIND_NAMES_ARG * bind_arg)
PT_NODE *spec, *prev_spec = NULL;
bool save_donot_fold;

/* for dblink: remote table's columns */
S_REMOTE_TBL_COLS *rmt_tbl_cols;

spec = scopes->specs;
scopes->specs = NULL;
while (spec)
Expand Down Expand Up @@ -1201,74 +1216,82 @@ pt_bind_scope (PARSER_CONTEXT * parser, PT_BIND_NAMES_ARG * bind_arg)
else if (table->node_type == PT_DBLINK_TABLE)
{
assert (spec->info.spec.derived_table_type == PT_DERIVED_DBLINK_TABLE);

int err;
REMOTE_COLS *remote;
PT_DBLINK_INFO *dblink_table = &table->info.dblink_table;

if (table->info.dblink_table.is_name && table->info.dblink_table.url == NULL)
{
int err;
S_REMOTE_TBL_COLS *rmt_tbl_cols;
REMOTE_COLS *remote;
PT_DBLINK_INFO *dblink_table = &table->info.dblink_table;

if (pt_resolve_dblink_server_name (parser, table, NULL) != NO_ERROR)
{
return;
}
}

/* remote table's column list */
rmt_tbl_cols = new (std::nothrow) S_REMOTE_TBL_COLS;
/* remote table's column list */
rmt_tbl_cols = new (std::nothrow) S_REMOTE_TBL_COLS;
if (rmt_tbl_cols == NULL)
{
PT_ERRORf (parser, table, "Failed to get column information for memory allocation error", ER_DBLINK);
return;
}

err = pt_dblink_table_get_column_defs (parser, table, rmt_tbl_cols);
err = pt_dblink_table_get_column_defs (parser, table, rmt_tbl_cols);

if (table->info.dblink_table.remote_table_name && *table->info.dblink_table.remote_table_name)
if (table->info.dblink_table.remote_table_name && *table->info.dblink_table.remote_table_name)
{
/* error from table-name@server */
if (err < 0)
{
/* error from table-name@server */
if (err < 0)
if (dblink_table->owner_name)
{
if (dblink_table->owner_name)
{
PT_ERRORf4 (parser, table,
"Failed to get column information for table [%s] on remote [%s].[%s]. err=%d",
dblink_table->remote_table_name,
dblink_table->owner_name->info.name.original,
dblink_table->conn->info.name.original, err);
}
else
{
PT_ERRORf3 (parser, table,
"Failed to get column information for table [%s] on remote [%s]. err=%d",
dblink_table->remote_table_name, dblink_table->conn->info.name.original, err);
}

return;
PT_ERRORf4 (parser, table,
"Failed to get column information for table [%s] on remote [%s].[%s]. err=%d",
dblink_table->remote_table_name,
dblink_table->owner_name->info.name.original,
dblink_table->conn->info.name.original, err);
}

if (pt_remake_dblink_select_list (parser, &spec->info.spec, rmt_tbl_cols) != NO_ERROR)
else
{
return;
PT_ERRORf3 (parser, table,
"Failed to get column information for table [%s] on remote [%s]. err=%d",
dblink_table->remote_table_name, dblink_table->conn->info.name.original, err);
}

goto error_exit;
}

/* error from dblink(server, 'select ... from ...' */
if (err < 0)
if (table->info.dblink_table.cols == NULL)
{
return;
if (pt_remake_dblink_select_list (parser, &spec->info.spec, rmt_tbl_cols) != NO_ERROR)
{
return;
Copy link
Contributor

Choose a reason for hiding this comment

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

This line must be 'goto error_exit;' , instead of 'return' to free rmt_tbl_cols.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok, good point.

}
}
}

dblink_table->remote_col_list = (void *) rmt_tbl_cols;
/* error from dblink(server, 'select ... from ...' */
if (err < 0)
{
goto error_exit;
}

/* save the column lists to free at end of parsing */
remote = (REMOTE_COLS *) parser_alloc (parser, sizeof (REMOTE_COLS));
if (remote == NULL)
{
PT_INTERNAL_ERROR (parser, "parser_alloc");
return;
}
dblink_table->remote_col_list = (void *) rmt_tbl_cols;

/* add dblink remote table's column list to Parser */
remote->cols = (void *) rmt_tbl_cols;
remote->next = parser->dblink_remote;
parser->dblink_remote = remote;
/* save the column lists to free at end of parsing */
remote = (REMOTE_COLS *) parser_alloc (parser, sizeof (REMOTE_COLS));
if (remote == NULL)
{
PT_INTERNAL_ERROR (parser, "parser_alloc");
goto error_exit;
}

/* add dblink remote table's column list to Parser */
remote->cols = (void *) rmt_tbl_cols;
remote->next = parser->dblink_remote;
parser->dblink_remote = remote;

table->info.dblink_table.cols =
parser_walk_tree (parser, table->info.dblink_table.cols, pt_bind_name_to_spec, spec, NULL, NULL);
}
Expand Down Expand Up @@ -1320,6 +1343,14 @@ pt_bind_scope (PARSER_CONTEXT * parser, PT_BIND_NAMES_ARG * bind_arg)
spec = prev_spec->next;
prev_spec->next = NULL;
}

return;

error_exit:
if (rmt_tbl_cols)
{
delete rmt_tbl_cols;
}
}

/*
Expand Down Expand Up @@ -3361,6 +3392,7 @@ pt_bind_names (PARSER_CONTEXT * parser, PT_NODE * node, void *arg, int *continue
{
/* It may be a generic function supported on the server. We put this case last so that user written
* methods will resolve before trying to make it a server function. */

if (!pt_type_generic_func (parser, node))
{
PT_NODE *top_node = NULL;
Expand Down Expand Up @@ -4836,7 +4868,8 @@ pt_dblink_table_fill_attr_def (PARSER_CONTEXT * parser, PT_NODE * attr_def_node,
int is_default = 0;

attr_def_node->data_type = NULL;
attr_def_node->type_enum = pt_type[attr->type_idx];
/* it needs to convert ext type to CCI_U_TYPE */
attr_def_node->type_enum = pt_type[(T_CCI_U_TYPE) CCI_GET_COLLECTION_DOMAIN (attr->type_idx)];
Copy link
Contributor

Choose a reason for hiding this comment

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

There is a dblink_get_basic_utype() function for a similar purpose. How about using this?
attr_def_node->type_enum = dblink_get_basic_utype(attr->type_idx)

switch (attr_def_node->type_enum)
{
case PT_TYPE_JSON:
Expand Down Expand Up @@ -5005,10 +5038,6 @@ pt_check_column_list (PARSER_CONTEXT * parser, const char *tbl_alias_nm, PT_DBLI
{
new_sel_list = new_sel_list ? parser_append_node (col, new_sel_list) : col;
}
else
{
parser_free_node (parser, col);
}
}

dblink_table->sel_list = new_sel_list;
Expand All @@ -5022,6 +5051,16 @@ pt_remake_dblink_select_list (PARSER_CONTEXT * parser, PT_SPEC_INFO * class_spec
PT_NODE *entity_name = dblink_table->qstr;
PT_NODE *range_var = dblink_table->qstr->next;

PT_NODE *attr_def_node = NULL;
PT_NODE *id_node;
PT_NODE *tmp;

PARSER_VARCHAR *var_buf = 0;

int error = NO_ERROR;

parser->custom_print |= PT_PRINT_SUPPRESS_FOR_DBLINK;

assert (dblink_table->qstr != NULL);
assert (class_spec->range_var != NULL);
assert (class_spec->only_all == PT_ONLY);
Expand All @@ -5033,11 +5072,11 @@ pt_remake_dblink_select_list (PARSER_CONTEXT * parser, PT_SPEC_INFO * class_spec
if (val == NULL)
{
PT_ERROR (parser, derived_table, er_msg ());
return ER_FAILED;
error = ER_DBLINK;
goto error_exit;
}

// select * from dblink_t1@remote_srv1;
PARSER_VARCHAR *var_buf = 0;
var_buf = pt_append_nulstring (parser, var_buf, "SELECT ");
var_buf = pt_append_varchar (parser, var_buf, pt_build_select_list_for_dblink (parser, dblink_table->sel_list));

Expand Down Expand Up @@ -5074,22 +5113,20 @@ pt_remake_dblink_select_list (PARSER_CONTEXT * parser, PT_SPEC_INFO * class_spec
PT_NODE *id_node = parser_new_node (parser, PT_NAME);
if (id_node == NULL)
{
return ER_FAILED;
error = ER_DBLINK;
goto error_exit;
}

id_node->info.name.original = "c";
if ((dblink_table->cols = pt_mk_attr_def_node (parser, id_node, NULL)) == NULL)
{
return ER_FAILED;
error = ER_DBLINK;
goto error_exit;
Copy link
Contributor

Choose a reason for hiding this comment

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

It seems necessary to restore parser->custom_print before returning at line 5127.
An easy way to do this is to 'goto error_exit' instead of 'return NO_ERROR'.
In this case, 'clean_up' or something similar can be a better name instead of the label 'error_exit' .

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok, good point.

}

return NO_ERROR;
}

PT_NODE *attr_def_node = NULL;
PT_NODE *id_node;
PT_NODE *tmp;

if (dblink_table->sel_list->node_type == PT_NAME && dblink_table->sel_list->type_enum == PT_TYPE_STAR)
{
assert (dblink_table->sel_list->next == NULL);
Expand All @@ -5098,13 +5135,15 @@ pt_remake_dblink_select_list (PARSER_CONTEXT * parser, PT_SPEC_INFO * class_spec
if ((id_node = parser_new_node (parser, PT_NAME)) == NULL)
{
PT_ERROR (parser, derived_table, er_msg ());
return ER_FAILED;
error = ER_DBLINK;
goto error_exit;
}

id_node->info.name.original = pt_append_string (parser, NULL, rmt_cols->get_name (i));
if ((tmp = pt_mk_attr_def_node (parser, id_node, rmt_cols)) == NULL)
{
return ER_FAILED;
error = ER_DBLINK;
goto error_exit;
}
attr_def_node = attr_def_node ? parser_append_node (tmp, attr_def_node) : tmp;
}
Expand All @@ -5119,14 +5158,18 @@ pt_remake_dblink_select_list (PARSER_CONTEXT * parser, PT_SPEC_INFO * class_spec

if ((tmp = pt_mk_attr_def_node (parser, id_node, rmt_cols)) == NULL)
{
return ER_FAILED;
error = ER_DBLINK;
goto error_exit;
}
attr_def_node = attr_def_node ? parser_append_node (tmp, attr_def_node) : tmp;
}
}

dblink_table->cols = attr_def_node;
return NO_ERROR;

error_exit:
parser->custom_print &= ~PT_PRINT_SUPPRESS_FOR_DBLINK;
Copy link
Contributor

Choose a reason for hiding this comment

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

Does the original value of parser->custom_print always have the bit PT_PRINT_SUPPRESS_FOR_DBLINK ?
Can we always delete the bit as this line does?
Storing the original value and restoring it at this line, which is a frequent pattern through out the code, seems more general way of handling parser->custom_print.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

printing the tree is called at various places through whole parser, compile, and optimize logic.
the custom_print should be kept the options to fit for purpose.

return error;
}

#define MAX_LEN_CONNECTION_URL 512
Expand Down Expand Up @@ -5154,33 +5197,14 @@ pt_dblink_table_get_column_defs (PARSER_CONTEXT * parser, PT_NODE * dblink, S_RE

if (table_name)
{
/* for collecting column info from "SELECT sel-list FROM table@server WHERE" */
int len = strlen (table_name);
char table_name_up[DB_MAX_IDENTIFIER_LENGTH * 2 + 1];
char user_name[DB_MAX_IDENTIFIER_LENGTH];

PARSER_VARCHAR *var_buf = 0;

/* preparing the query to get the column info */
var_buf = pt_append_nulstring (parser, var_buf, "SELECT ");
var_buf = pt_append_varchar (parser, var_buf, pt_build_select_list_for_dblink (parser, dblink_table->sel_list));
var_buf = pt_append_nulstring (parser, var_buf, " FROM ");

/* make it upper case for Oracle */
intl_identifier_upper (table_name, table_name_up);
/* all attr's from "SELECT * FROM" */
rmt_tbl_cols->set_select_star ();

/* make "user-name"."table-name" for reserved word */
find = strchr (table_name_up, '.');
if (find)
{
snprintf (user_name, (int) (find - table_name_up) + 1, "%s", table_name_up);
sprintf (t_name, "\"%s\".\"%s\"", user_name, find + 1);
}
else
{
sprintf (t_name, "\"%s\"", table_name_up);
}
var_buf = pt_append_nulstring (parser, var_buf, t_name);
/* preparing the query to get the column info */
var_buf = pt_append_nulstring (parser, var_buf, "SELECT * FROM ");
var_buf = pt_append_nulstring (parser, var_buf, table_name);
sql = (char *) var_buf->bytes;
}
else
Expand Down Expand Up @@ -5221,7 +5245,7 @@ pt_dblink_table_get_column_defs (PARSER_CONTEXT * parser, PT_NODE * dblink, S_RE
{
/* this can not be reached, something wrong */
er_set (ER_ERROR_SEVERITY, ARG_FILE_LINE, ER_DBLINK, 1, "unknown error");
res = ER_FAILED;
res = ER_DBLINK;
goto set_parser_error;
}

Expand All @@ -5236,11 +5260,6 @@ pt_dblink_table_get_column_defs (PARSER_CONTEXT * parser, PT_NODE * dblink, S_RE
res = NO_ERROR;

set_parser_error:
if (res < 0)
{
delete rmt_tbl_cols;
}

if (req >= 0)
{
int err;
Expand Down Expand Up @@ -11473,6 +11492,7 @@ pt_get_column_name_pre (PARSER_CONTEXT * parser, PT_NODE * node, void *arg, int
{
check_for_already_exists (parser, plkcol, NULL, node->info.name.original);
}
plkcol->col_list->info.name.flag = node->info.name.flag;
break;

case PT_VALUE:
Expand Down Expand Up @@ -11611,6 +11631,11 @@ pt_check_dblink_column_alias (PARSER_CONTEXT * parser, PT_NODE * dblink)
return ER_FAILED;
}

if (rmt_tbl_cols->is_select_star ())
{
return NO_ERROR;
}

cols = dblink->info.dblink_table.cols;
while (cols)
{
Expand Down
Loading