fsprojects / Rezoom.SQL

Statically typechecks a common SQL dialect and translates it to various RDBMS backends
MIT License
669 stars 25 forks source link

better exception message when sql contains qualified table name #47

Closed smoothdeveloper closed 5 years ago

smoothdeveloper commented 5 years ago
type Q = SQL<"select * from a.b">

before:

key not found

after:

Schema 'a' doesn't exist. Existing schemas: 'Main', 'Temp'.

smoothdeveloper commented 5 years ago

I guess this should ideally happen/be reported earlier during the parsing but I think @rspeele will know better how to do this.

Bare exceptions don't move well through the TP if their message is not enhanced for the developer who reads compiler errors.

This library has in fact excellent error reporting, but I just hit that issue with no informative error message.

Idea for the enhancing error messages: to report similar names for invalid identifers to help developer fix typos, the same way as fsc.exe does when you mistyping an identifier.

rspeele commented 5 years ago

Good fix. Look for uses of the ‘failAt’ helper to see how to produce an error msg with a reference to the location in the SQL source.

smoothdeveloper commented 5 years ago

Thanks for the pointer @rspeele, also, the schema handling is an area I'm interested about wrt your library.

I'm considering trying it out on some large existing databases, and access to each schema will be a requirement in my case.

If the work to bring schema is not too daunting from current state (I've seen it is already considered to a good degree to distinguish temp and main), I'll consider digging further in the code beside looking at making the error message better for this PR via failAt.

smoothdeveloper commented 5 years ago

@rspeele, I have made all the updates I could think of for this check, let me know if you see anything to adjust.

rkosafo commented 5 years ago

@rspeele No. I only have to pull the changes to the standard branch.

smoothdeveloper commented 5 years ago

I'll be giving a look at handling create schema: https://github.com/smoothdeveloper/Rezoom.SQL/tree/initial-schema-support

@rspeele would you be OK to draft some of the requirements if we were to add this feature to the library? I'm unsure about the drop / rename stuff in case of schema, and I'm not sure SQLite supports schemas, for this backend we can mangle (prettily) object names?

rspeele commented 5 years ago

Hey,

Your work on schema support looks good.

Pretty sure SQLite doesn't have schemas. However, I wouldn't worry about that too much. The approach I have taken in the past is to simply throw an error when doing something the backend doesn't support, rather than doing complex work to simulate the feature's existence. SQLite also doesn't support "ALTER TABLE" and simulating it would be a mess. Probably the biggest exception to that is the trickery around pretending T-SQL supports boolean values, but in that case, the work to throw errors on improper uses of booleans was practically the same effort as wrapping those in CASE exprs or with (x<>0).