[CBRD-24867] rev: regression bug-fix related to DBLINK - CCI_U_TYPE, no serve name #4507
Conversation
…ix-cci-utype-no-server-name
|
Can you give test cases for each regression? They would help me to understand the changes in this PR. |
see the last comment on JIRA issue CBRD-24867. |
src/parser/name_resolution.c
Outdated
| return; | ||
| if (pt_remake_dblink_select_list (parser, &spec->info.spec, rmt_tbl_cols) != NO_ERROR) | ||
| { | ||
| return; |
There was a problem hiding this comment.
This line must be 'goto error_exit;' , instead of 'return' to free rmt_tbl_cols.
src/parser/name_resolution.c
Outdated
| } | ||
|
|
||
| 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; |
There was a problem hiding this comment.
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' .
| return NO_ERROR; | ||
|
|
||
| error_exit: | ||
| parser->custom_print &= ~PT_PRINT_SUPPRESS_FOR_DBLINK; |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
| pt_append_string (parser, derived_spec->info.dblink_table.remote_table_name, | ||
| class_spec_info->entity_name->info.name.original); | ||
|
|
||
| parser->custom_print &= ~PT_PRINT_SUPPRESS_FOR_DBLINK; |
There was a problem hiding this comment.
Can the original value of parser->custom_print have PT_PRINT_SUPPRESS_FOR_DBLINK bit?
If it can, deleting the bit does not always restore the original value.
There was a problem hiding this comment.
set and clear is always pair, so the flag is not set erroneously.
src/query/execute_statement.c
Outdated
| @@ -13687,20 +13692,21 @@ do_execute_insert (PARSER_CONTEXT * parser, PT_NODE * statement) | |||
|
|
|||
| CHECK_MODIFICATION_ERROR (); | |||
|
|
|||
| if (statement->xasl_id == NULL) | |||
| /* for dblink: no need to check xal_is is NULL */ | |||
There was a problem hiding this comment.
ah ha, mis-spelled
| @@ -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)]; | |||
There was a problem hiding this comment.
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)
http://jira.cubrid.org/browse/CBRD-24867
buf-fix list
ext-type should be converted to CCI_U_TYPE
we should consider the query with URL instead server-name
not need to check for table@server (i.e. SELECT * FROM ...)
added m_attr_star to S_REMOTE_TBL_COLS to identified (SELECT * FROM...)
only when the column-def list not exists
the quoted name like "From" is transmitted to "From" as a quoted name
because the remote query should be executed on other DBMS like Oracle.
The ["From"] and `"From"` and """From""" is will be "From" at remote server even though the DBMS is CUBRID or not.
the parser set the flag cannot_prepare not to prepare for genric-function
we should clear the flag for remote DML.