search

databrickslabs / remorph

Cross-compiler and Data Reconciler into Databricks Lakehouse
Other
39 stars 25 forks source link

Unresolved Bang #880

Closed sundarshankar89 closed 2 months ago

sundarshankar89 commented 2 months ago

This adds a lexer rule to catch any or all unresolved bang into a bucket unresolvedSnowSqlCommand if this is not implemented the grammar for let creates them as unresolvedCommand which is incorrect assumption.

github-actions[bot] commented 2 months ago

Coverage tests results

430 tests  ±0   382 :white_check_mark: ±0   6s :stopwatch: -1s   5 suites ±0     0 :zzz: ±0    5 files   ±0    48 :x: ±0 

For more details on these failures, see this check.

Results for commit 3d299d8e. ± Comparison against base commit 6794006d.

:recycle: This comment has been updated with latest results.

sundarshankar89 commented 2 months ago

I do not believe that this will work sundar. I am also of the opinion that we are expecting valid input to the parser such that:

!badcommand something something

Is OK to reject with a syntax error on the !

I am working on a tool chain that will stop when syntax errors are detected and we would not try to generate IR using an incorrect ParseTree, which is what we are doing in places now.

I don't think that we need this change. The CALL issue is a big problem actually - they implemented that without regard to anything else.

@nfx how do you want to tackle this current framework resolves them unresolvedCommand this rule is added to circumvent that, I agree with the approach but will require separate PR, Can we take this in as Tech Debt and tackle once we have our functional tests running as expected.

sundarshankar89 commented 2 months ago

@nfx as discussed I will close this and remove the invalid bang SQL command from test case