[FRG-21] Data types are not validated outside of a DDL context

[dynamobi-build]

[reporter="angel", created="Mon, 16 Jan 2006 21:18:58 -0500 (GMT-05:00)"] Outside of a DDL context, things like precision and scale for a data type are not validated.

For instance, the validator doesn't catch expressions such as "cast (true as boolean(0))". The validator should indicate that the boolean data type does not take a precision.

Looks like SqlValidatorImpl.validateDataType need to share some code with DdlHandler.

[dynamobi-build]

[author="jvs", created="Tue, 17 Jan 2006 11:25:26 -0500 (GMT-05:00)"] Copying some relevant history from dtbug 78 "cast(x as int) doesn't work":

Description: [reply] Opened: 2004-06-22 20:54

Instead, you have to use cast(x as integer). The reason is that
SqlDataType.deriveType relies on SqlTypeName for the list of supported types,
which is wrong. It should be relying on the catalog, same as CREATE TABLE. The
code we need is in DdlValidator.findSqlDataType. Obviously, SqlDataType can't
call to this Farrago code; it will need to call indirectly via a virtual method
in SqlValidator instead. However, DdlValidator won't be available while
validating a non-DDL statement. So we need to share the lookup method with
FarragoPreparingStmt. (As a matter of fact, a bunch of stuff should be shared
between DdlValidator and FarragoPreparingStmt, so now might be a good time to
set that up.)


------- Comment #1 From Lee Schumacher 2004-06-26 11:21 [reply] -------

I think char/character has similar issues.

[dynamobi-build]

[author="jvs", created="Sun, 25 Jun 2006 18:41:46 -0500 (GMT-05:00)"] As far as I can tell, the common logic that needs to be shared is all in DdlHandler.validateTypedElement. We should move it to a new method FarragoSessionStmtValidator.validateDataType(SqlDataTypeSpec). For DDL, call this from DdlHandler.validateTypedElement. For non-DDL, call this from FarragoSqlValidator.validateDataType, which already overrides SqlValidatorImpl.validateDataType. Some of the logic will need to be tweaked (e.g. looking up the CwmSqlsimpleType by name instead of starting with it).

For DdlHandler.validateTypedElement, I think it should be safe to assume that if validator.getSqlDefinition returns null, then it means we're revalidating a previously saved element, implying that we can skip type validation altogether. (If this isn't true, then we'll need a conversion from FemSqltypedElement to SqlDataTypeSpec, which can be achieved by composition of FarragoTypeFactory.createCwmElementType and SqlTypeUtil.convertTypeToSpec, but that seems pointless, because I think createCwmElementType assumes the input has already been validated.)

It would be nice to deal with UDT's inside of FarragoStmtValidator.validateTypedElement, even though other stuff downstream (like doing the right thing for casting a builtin to a UDT) may or may not be working. One complication with this is that in this case we need to make FarragoSqlValidator call FarragoPreparingStmt.addDependency. That way a view which contains CAST(x AS ip_address) will get a catalog dependency on the ip_address UDT (very important). But that can't be done from inside FarragoStmtValidator (only FarragoSqlValidator knows about FarragoPreparingStmt), so it has to be done as a separate step in FarragoSqlValidator.validateTypedElement.

Some of the code which needs to stay in DdlHandler.validateTypedElement (at least for now) includes

• creating dependencies on UDT's (this is done differently for DDL)
  
• dealing with CWM length/precision discrimination
  
• dealing with row types (since SqlDataTypeSpec doesn't support this yet)
  
  You should be able to clean it up to use a single generic catch to annotate any specific exception with the DDL position information (nicer than what's happening currently with the wrapping happening for each problem individually).
  
  My hypocritical DdlHandler.validateTypedElement TODO from long ago says: "break this method up". Please do as I say, not as I do. :P
  

[dynamobi-build]

[author="jvs", created="Sun, 25 Jun 2006 18:47:58 -0500 (GMT-05:00)"] See also FRG-153.

[dynamobi-build]

[author="angel", created="Tue, 11 Jul 2006 09:53:37 -0500 (GMT-05:00)"] Fixed in eigenchange 7103.