You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@age.apache.org by GitBox <gi...@apache.org> on 2022/02/11 05:05:24 UTC
[GitHub] [incubator-age] emotionbug opened a new pull request #185: feat: Introduce `enable_auto_column_conversion`
emotionbug opened a new pull request #185:
URL: https://github.com/apache/incubator-age/pull/185
This is used for untyped cypher clauses. The current cypher query must be of a type to be used, which requires a lot of work from users who only use cypher.
In addition, it is thought that many functional implementations will be needed to determine what data is returned when implementing CLI functions in the future.
Thus, it is implemented in this way and is designated as GUC's 'age.enable_auto_column_conversion', and the default value is FALSE.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@age.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-age] emotionbug commented on a change in pull request #185: feat: Introduce `enable_auto_column_conversion`
Posted by GitBox <gi...@apache.org>.
emotionbug commented on a change in pull request #185:
URL: https://github.com/apache/incubator-age/pull/185#discussion_r807625467
##########
File path: src/include/parser/cypher_parse_node.h
##########
@@ -49,7 +54,6 @@ typedef struct cypher_parsestate
* else). It is only used by transform_cypher_item_list.
*/
bool exprHasAgg;
- bool p_opt_match;
Review comment:
No longer used.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@age.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-age] emotionbug commented on a change in pull request #185: feat: Introduce `enable_auto_column_conversion`
Posted by GitBox <gi...@apache.org>.
emotionbug commented on a change in pull request #185:
URL: https://github.com/apache/incubator-age/pull/185#discussion_r807441412
##########
File path: src/backend/parser/cypher_analyze.c
##########
@@ -612,10 +623,18 @@ static Query *analyze_cypher_and_coerce(List *stmt, RangeTblFunction *rtfunc,
// check the number of attributes first
if (attr_count != rtfunc->funccolcount)
{
- ereport(ERROR,
- (errcode(ERRCODE_DATATYPE_MISMATCH),
- errmsg("return row and column definition list do not match"),
- parser_errposition(pstate, exprLocation(rtfunc->funcexpr))));
+ if (age_guc_enable_auto_column_conversion)
Review comment:
Regarding this logic, it's a little uncomfortable while using it, so I'll revise it as follows.
If the corresponding flag is set, it automatically matches regardless of the number of columns, otherwise, if the number of columns does not match, it will cause an error as before.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@age.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-age] JoshInnis commented on a change in pull request #185: feat: Introduce `enable_auto_column_conversion`
Posted by GitBox <gi...@apache.org>.
JoshInnis commented on a change in pull request #185:
URL: https://github.com/apache/incubator-age/pull/185#discussion_r807426247
##########
File path: src/include/parser/cypher_parse_node.h
##########
@@ -26,6 +26,12 @@
#define AGE_DEFAULT_ALIAS_PREFIX "_age_default_alias_"
#define AGE_DEFAULT_VARNAME_PREFIX "_age_varname_"
+typedef struct extends_parsestate
Review comment:
This needs documentation to explain what it is.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@age.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-age] JoshInnis commented on a change in pull request #185: feat: Introduce `enable_auto_column_conversion`
Posted by GitBox <gi...@apache.org>.
JoshInnis commented on a change in pull request #185:
URL: https://github.com/apache/incubator-age/pull/185#discussion_r807420972
##########
File path: src/backend/parser/cypher_analyze.c
##########
@@ -19,7 +19,6 @@
#include "postgres.h"
-#include "catalog/pg_type_d.h"
Review comment:
I got a compile error. I had to add #include "catalog/pg_type.h" to get to work
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@age.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-age] emotionbug commented on a change in pull request #185: feat: Introduce `enable_auto_column_conversion`
Posted by GitBox <gi...@apache.org>.
emotionbug commented on a change in pull request #185:
URL: https://github.com/apache/incubator-age/pull/185#discussion_r807441412
##########
File path: src/backend/parser/cypher_analyze.c
##########
@@ -612,10 +623,18 @@ static Query *analyze_cypher_and_coerce(List *stmt, RangeTblFunction *rtfunc,
// check the number of attributes first
if (attr_count != rtfunc->funccolcount)
{
- ereport(ERROR,
- (errcode(ERRCODE_DATATYPE_MISMATCH),
- errmsg("return row and column definition list do not match"),
- parser_errposition(pstate, exprLocation(rtfunc->funcexpr))));
+ if (age_guc_enable_auto_column_conversion)
Review comment:
Regarding this logic, it's a little uncomfortable while using it, so I'll revise it as follows.
If the corresponding flag is set, it automatically matches regardless of the number of columns, otherwise, if the number of columns does not match, it will cause an error as before.
Before:
cypher('graph', $$MATCH (n) RETURN n$$) as (ignore agtype)
=> Columns ( ignore )
After:
cypher('graph', $$MATCH (n) RETURN n$$) as (ignore agtype)
=> Columns ( n )
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@age.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-age] JoshInnis commented on a change in pull request #185: feat: Introduce `enable_auto_column_conversion`
Posted by GitBox <gi...@apache.org>.
JoshInnis commented on a change in pull request #185:
URL: https://github.com/apache/incubator-age/pull/185#discussion_r807414489
##########
File path: src/backend/parser/cypher_analyze.c
##########
@@ -612,10 +623,18 @@ static Query *analyze_cypher_and_coerce(List *stmt, RangeTblFunction *rtfunc,
// check the number of attributes first
if (attr_count != rtfunc->funccolcount)
{
- ereport(ERROR,
- (errcode(ERRCODE_DATATYPE_MISMATCH),
- errmsg("return row and column definition list do not match"),
- parser_errposition(pstate, exprLocation(rtfunc->funcexpr))));
+ if (age_guc_enable_auto_column_conversion)
Review comment:
Should the functionality be: if the column definition list doesn't match: check the flag, or if the flag is enabled don't check the column definition list?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@age.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org