Support anonymous blocks for new Oracle dialect #53
Conversation
|
@MasterOdin be good to get eyes on this briefly before merging, but it's working. Going to use in BKS either way |
| }); | ||
|
|
||
| describe('identifying statements with anonymous blocks', () => { | ||
| it('should identify a create table then a block', () => { |
There was a problem hiding this comment.
why have this test repeated here and in parser/oracle.spec.ts? Given that identify is a relative shallow operation over the result of the parser, I'd rather just beef up the asserts there, and get rid of this test.
There was a problem hiding this comment.
Just as an idiot check to be honest. Costs very little, gives me some assurances
There was a problem hiding this comment.
For example, I changed the execution type that this returns, the tests reflect that. Nothing wrong with over testing.
test/parser/oracle.spec.ts
Outdated
| expect(result.body[0].type).to.eq('ANON_BLOCK'); | ||
| expect(result.body[0].start).to.eq(0); | ||
| expect(result.body[0].end).to.eq(120); | ||
| expect(result.body.length).to.eql(2); |
There was a problem hiding this comment.
I think this should be the first assert where if there were no statements found, then we'd get an undefined error for a test failure, which isn't as meaningful imo.
There was a problem hiding this comment.
Yes I reorganized this incorrectly, moving the other way around
|
I'll address these couple of issues, then I'll merge it. |
|
@MasterOdin I've added a few changes:
I don't think this change requires a major version bump, as the new execution and statement types are only valid for Oracle, so no behavior has changed for any other databases. |
|
@MasterOdin I'll merge this tomorrow if you don't have any other changes. |
MasterOdin
changed the title
Oracle supports statement-level blocks (because...reasons)
This adds support for them.
It also adds oracle as a dialect