Parse() then AST.toString() doesn't restore square brackets on column name

[andymcmullan]

If a column name requires square brackets, e.g. due to a space or a period in the column name, toString() on the AST loses the brackets resulting in incorrect SQL.

For example

const ast = alasql.parse('SELECT [Foo Bar] FROM tbl');
console.log(ast.toString());

gives SELECT Foo Bar FROM tbl instead of SELECT [Foo Bar] FROM tbl

Similarly:

const ast = alasql.parse('SELECT [Foo.Bar] FROM tbl');
console.log(ast.toString());

gives SELECT Foo.Bar FROM tbl instead of SELECT [Foo.Bar] FROM tbl

[vishal6557]

I'm interested in working on this. Thanks!

[vishal6557]

Any pointer, where should I start? Spend some time on but kind of tricky to find where the '[' is strip off.

[mathiasrw]

Lovely!

Have a look at BRALITERAL in the parser

This is where they are matched and below is where we cut off the information. It is cut out before we generate the AST.

The easy part is to edit line 357 to something like

{ $$ = {val:doubleq($1.substr(1,$1.length-2)), wrap:$1.substr(0,1)}; }

The hard part is to edit the code to use the new location of the value - and to make sure it works when we have a string that is not wrapped.

Use yarn jison when you changes in the .jison file.

I propose running yarn gulp && mocha ./test --bail to identify the places one at a time where we need to update the code.

[Saurabhmahto]

If the issue is not resolved can i work on this? If yes pls tell me how can i run/test it in local machine.

[Saurabhmahto]

As a beginner in open source, after editing line 357 as mentioned, how can I identify the other locations that require changes? Please help.

[mathiasrw]

@Saurabhmahto Nice work getting into the flow.

Are the tests passing? Use yarn jison to build the grammar, and then build and test using yarn test --bail

[Saurabhmahto]

I made changes on line 357 and build the grammer using yarn jison. After implementing the changes, I rebuilt the project and ran the test suite using yarn test --bail. However, I encountered a failing test case that needs further investigation.

pls guide me further

[mathiasrw]

Lovely.

The test is in https://github.com/AlaSQL/alasql/blob/develop/test/test046.js#L33

It is probably the [...] part that is not working. I propose you look for the most simple thing you can write that breaks it. It might be something like select 1 as [abc]

There are many ways to do this, but one way could be to step though alasql while it is running:

• Make a file named poc1740.js
  • require the file dist/alasql.fs.js
  • console log the result of alasql('select 1 as [abc]') (or if you find another good example)
• Run node poc1740.js
  • Check if you get some sort of warning or error that can help indicate where the problem is
    • if so insert the line break; before that code
• Now inspect the running code
  • Run node --inspect-brk poc1740.js
  • Go to chrome://inspect in chrome (yes, that exact url)
  • Click the link under "target"
  • now step all your way to understand the flow and see if you can nail when the problem happens. We are looking for the code that is seeking to find the value of from the AST. It is expecting the value to be a string, but the data is an object (the change you made in the jison)

Note that when you are debugging it is faster / easier to edit directly in the dist/alasql.fs.js file to insert breaks and "run" to them. But the changes are reset when you build again next time.

ping me if you get stuck...

[akashyadav24]

Hello @mathiasrw
I have fixed this Opened a pull request
can you please review it