FWD » Core Development » Database

• if the immediate child is a NUM_LITERAL, it will add a space before and after the text node.
• when using functions, it will usually add a comma but if the first child of the AST is a SELECT, it won't happen.
• Another case where if the parent is a SUBSELECT, the NUM_LITERAL shouldn't be able to fall in the default case.
  The change to FqlToSqlConverter is to prevent generating a column name for the NUM_LITERAL.

I've initially tried to make use of FqlToSqlConverter to generate the subselect statement, but it may be too late to handle it here.
Please review the current changes.

Review of #7235 patch in #7235-3.

• When you append the NUM_LITERAL, you also append SELECT (which is OK). You use subSelectNode.graftAt(selectValueNode, 0); and next.graftAt(selectNode, 0);. Is subSelectNode.graftAt(selectNode, 0); OK, for the select token? Maybe next = subSelectNode, but I doubt this as "exists".equals(next.getText()) && next.getFirstChild().getType() == SUBSELECT. Please check that the SELECT keyword is placed correctly (as a left sibling of the num literal, as a child of the subselect).
• Don't inject SELECT and NUM_LITERAL if SELECT already exists! You may already preprocess an FQL which is of type EXISTS (SELECT 1 FROM tt); you don't want to end up with EXISTS (SELECT 1 SELECT 1 FROM tt).
• I think you can drop child.getType() != NUM_LITERAL in FqlToSqlConverter. It is not that bad to have SELECT 1 as col1 from tt.
• Don't use if (ast.getFirstChild().getType() != SELECT) to rule out the commas, as they are used to separate a functions parameters. Your problem is somewhere else regarding commas. MY_FUNC((SELECT 1 FROM tt), 2) is a valid syntax, where the subselect is a function argument. Your conditional removes the comma between the first and second arguments. Maybe you can use nextChild with SUBSELECT to check which children are you iterating:
  

  // SUBSELECT case
  Aast child = ast.getChildAt(index);
  // this only handled num literals as selection fields; this can be extended to support booleans/string/table fields, etc.
  if (child.getType() == NUM_LITERAL)
  {
      // add comma only if (ast.getChildAt(index + 1) != FROM)
      // add space
  }
  

• Let the NUM_LITERAL be emitted by the default case (buf.append(text))

Please address the first point. That is, make sure SELECT, NUM_LITERAL and FROM are on the same level, children of SUBSELECT.
Afterward, check that injecting two NUM_LITERAL (EXISTS(SELECT 1, 1 FROM TT)) produces the right FQL (with proper spacing and commas). This should be work.

Alexandru Lungu wrote:

... Please check that the SELECT keyword is placed correctly (as a left sibling of the num literal, as a child of the subselect).

I reorganized the nodes since they were not placed correctly.

... Your conditional removes the comma between the first and second arguments. Maybe you can use nextChild with SUBSELECT to check which children are you iterating: ...

I took the code suggestion and changed it a little bit. Instead of checking the next node and see if it's a FROM, I check the previous node for a SELECT, this way I can generate statements like select 1, 2, 3 from. At the same time, another space needs to be added when if the child is a FROM in order to avoid obtaining select 1, 2, 3from.

I attached a new patch where I made the suggested changes, please review.

Added this patch to 7026c.
Tested this. I got a -0.24% performance improvement with a 7026c profile run.
Just rebased 7026c with trunk.

Fully reviewed tested and profiled. 7026c is ready for trunk.
I intent to bump the last FWD-H2 revision with 7026c (rev. 17).

Merged 7026c to trunk as rev. 14553.