Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

PostgreSQL: Strict typing requires explicit type casts #2213

Open
homedirectory opened this issue Mar 20, 2024 · 2 comments · May be fixed by #2234
Open

PostgreSQL: Strict typing requires explicit type casts #2213

homedirectory opened this issue Mar 20, 2024 · 2 comments · May be fixed by #2234

Comments

@homedirectory
Copy link
Contributor

homedirectory commented Mar 20, 2024

Description

A problem with execution of EQL queries against PostgreSQL has been identified.
Queries that get compiled to the UNION SQL statements are affected. This includes:

  • Query sources that are composed of other queries (aka nested SELECT).

    Relevant classes:

    • ua.com.fielden.platform.eql.stage3.sources.Source3BasedOnQueries

The core of the problem stems from the way Type Resolution for UNION is designed in PostgreSQL.

In short:

  1. PostgreSQL represents multiple UNIONs as a binary tree with a single growing side.
  2. If all parts of UNION are of type unknown, resolve as type text.

For example,

SELECT NULL 
UNION 
SELECT NULL 
UNION 
SELECT 1;

is represented by the following S-expression:

(
 ((SELECT NULL) (SELECT NULL)) ; text
 (SELECT 1) ; integer
)

which leads to ERROR: UNION types text and integer cannot be matched.

This problem can be solved by inserting explicit type casts where there are multiple unions.
Considering the above example, the following version would work correctly:

SELECT (CAST NULL AS integer)
UNION 
SELECT NULL -- optionally cast here too
UNION 
SELECT 1;

Insertion of explicit type casts could be achieved by performing additional analysis during the
transformation of EQL into SQL.

Expected outcome

UNION statements for PostgreSQL are generated with explicit type casts inserted where necessary.

@homedirectory homedirectory self-assigned this Mar 20, 2024
homedirectory added a commit that referenced this issue Mar 20, 2024
homedirectory added a commit that referenced this issue Mar 21, 2024
…formation

Field `dbVersion: DbVersion` was removed due to it being inferable from
a `Dialect` instance.

This change provides access to more information about the active SQL
dialect. For example, we get access to Hibernate's knowledge of SQL type
names, which is required to solve the problem described in this issue.
homedirectory added a commit that referenced this issue Mar 21, 2024
homedirectory added a commit that referenced this issue Mar 22, 2024
homedirectory added a commit that referenced this issue Mar 22, 2024
* Increase immutability
* Be concise
* Add documentation and comments
homedirectory added a commit that referenced this issue Mar 22, 2024
Needed additional code to overcome limitations of Hibernate
homedirectory added a commit that referenced this issue Mar 22, 2024
homedirectory added a commit that referenced this issue Mar 27, 2024
@homedirectory
Copy link
Contributor Author

homedirectory commented Apr 2, 2024

Further research revealed that the type mismatch problem is more general and is not limited to the UNION case only.
PostgreSQL's approach to data types is fundamentally different from the approach of SQL Server. PostgreSQL enforces strict typing rules.

Nevertheless, instead of tackling the problem in its full generality, it might be sufficient to cover the following cases:

  1. UNION query (as described in the issue)

  2. Subquery

    PostgreSQL resolves output column types of a subquery as soon as it finishes parsing the subquery.
    One consequence of this is that any NULLs returned by a subquery are resolved as text.

    One kind of queries affected by this is "ID-only" queries in EQL, i.e., queries with an entity result type that yield a single column -- ID.

    An ID-only query compiles down to:

    SELECT * from <entity> where id in (select <yielded-value> from <entity>)

    Entity IDs have a numeric type, so the result of subquery (select <yielded-value> from <entity>) must also be a numeric type. However, were <yielded-value> to be NULL, such an
    expression would be considered illegal by PostgreSQL because the subquery's resulting type would
    be resolved to text which can't be matched with a numeric type.

    Another example of a subquery returning NULL that gets resolved as text:

     SELECT a 
     FROM (SELECT 2    AS a) AS t1
     JOIN (SELECT NULL AS b) AS t2
     ON t1.a = t2.b;
  3. CASE expression - CASE is subject to the same type resolution rules as UNION.

    Here are some examples that PostgreSQL doesn't like:

    -- fails
    SELECT 1 WHERE 1=(SELECT CASE WHEN 1=1 THEN NULL ELSE NULL END);
    -- works
    SELECT 1 WHERE 1=(SELECT CAST ((CASE WHEN 1=1 THEN NULL ELSE NULL END) AS integer));
  4. LIKE operator

    target LIKE pattern
    

    PostgreSQL rejects any usage of LIKE where the left-hand side (target) is not a string.
    This is unlike SQL Server that performs implicit type conversion of both target and pattern to a string type.
    Converting pattern is not desirable as it may lead to subtle errors.
    Converting target, however, is a sensible approach.

@homedirectory
Copy link
Contributor Author

homedirectory commented Apr 3, 2024

3d91a167b7 introduces changes that need to be followed up by recompilation of the tg-testing module.

homedirectory added a commit that referenced this issue Apr 5, 2024
This is to avoid 2 placeholder types from different contexts
being accidentally considered equal.
homedirectory added a commit that referenced this issue Apr 5, 2024
- Add documentation
- Implement toString() methods to facilitate reasoning
- Reduce visibility of PropType constructor (static method should be used)
homedirectory added a commit that referenced this issue Apr 5, 2024
…ions

This is achieved by casting the NULL to the type of the other operand
homedirectory added a commit that referenced this issue Apr 9, 2024
homedirectory added a commit that referenced this issue Apr 9, 2024
In general, this approach of asserting that an exception is thrown is
far from the best because it ignores potential exceptions that may be
thrown for unexpected reasons. Being more specific and expecting only a
certain type of exceptions doesn't work either since the exception gets
rethrown at some higher level, effectively becoming a cause of another
exception. Should we be making assertions about the stack trace at this
point? I don't think so.
@homedirectory homedirectory changed the title PostgreSQL: UNION types cannot be matched PostgreSQL: Strict typing requires explicit type casts Apr 10, 2024
homedirectory added a commit that referenced this issue Apr 10, 2024
The converstion itself was also slightly modified:
* Use an unbounded VARCHAR by default instead of setting the limit to
  255 chars, which has no clear basis.
* Optimise for Integers by limiting the VARCHAR length to 10 chars.
homedirectory added a commit that referenced this issue Apr 10, 2024
And for all DB back-ends, not just PostgreSQL and SQL Server.
homedirectory added a commit that referenced this issue Apr 10, 2024
… with string type

Wheels of EQL3 are turning so the workaround is no longer needed.
Moreover, we can now be confident that the null type is never taken for
the string type by mistake, which is vastly important for PostgreSQL.
homedirectory added a commit that referenced this issue Apr 11, 2024
* Use a sealed interface with inner classes to facilitate pattern
  matching with switch.
* Instead of inserting type casts conditionally, depending on the
  database, always insert them. The conditional approach likely relied
  on implicit type conversion performed by some DBs. Although,
  endAsInt() worked correctly only with H2, which had gone unnoticed as
  its usage has been scarce.
homedirectory added a commit that referenced this issue Apr 11, 2024
This assertion reports list contents in case of failure.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant