jrgemignani commented on code in PR #1237:
URL: https://github.com/apache/age/pull/1237#discussion_r1362465669
##########
src/backend/parser/cypher_gram.y:
##########
@@ -1543,12 +1543,10 @@ expr:
* supported
*/
else
- {
ereport(ERROR,
(errcode(ERRCODE_SYNTAX_ERROR),
errmsg("invalid indirection syntax"),
ag_scanner_errposition(@1, scanner)));
- }
Review Comment:
Why remove the surrounding braces?
##########
src/backend/parser/cypher_expr.c:
##########
@@ -251,14 +251,11 @@ static Node *transform_A_Const(cypher_parsestate
*cpstate, A_Const *ac)
d = boolean_to_agtype(boolVal(&ac->val));
break;
default:
- if (ac->isnull)
- {
Review Comment:
Why are the braces being removed?
##########
src/backend/utils/adt/agtype.c:
##########
@@ -1965,7 +1965,7 @@ Datum _agtype_build_path(PG_FUNCTION_ARGS)
*/
if (nargs >= 1 && nargs <= 3)
{
- int i = 0;
+ i = 0;
Review Comment:
If this `i` is only used within this `if` block, then let's only have it
defined here. If this `i` is used outside of this `if` block for other things,
then let's change the interior `i` to some other variable name. Let's try to
keep the scope of the variable as close to where it is used and let's not
duplicate variables unless absolutely necessary.
##########
src/backend/parser/cypher_gram.y:
##########
@@ -1543,12 +1543,10 @@ expr:
* supported
*/
else
- {
Review Comment:
Why remove the surrounding braces?
##########
src/backend/parser/cypher_expr.c:
##########
@@ -251,14 +251,11 @@ static Node *transform_A_Const(cypher_parsestate
*cpstate, A_Const *ac)
d = boolean_to_agtype(boolVal(&ac->val));
break;
default:
- if (ac->isnull)
- {
+ if (ac->isnull) {
Review Comment:
Braces do *not* go at the end of the line of code, they go on the following
line.,
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
