Disable incorrect character pushdown in PostgreSQL #7145
Conversation
findepi
force-pushed
the
findepi/disable-incorrect-character-pushdown-in-postgresql-eaa946
branch
from
7a12107
to
b19a21a
Compare
|
Turns out postgresql supports varchars and chars longer than Trino, so brought back the logic for this. |
findepi
force-pushed
the
findepi/disable-incorrect-character-pushdown-in-postgresql-eaa946
branch
from
b19a21a
to
36f2403
Compare
|
Had to rebase onto master, to rebase #6874, to resolve conflict, to have test run. |
plugin/trino-postgresql/src/main/java/io/trino/plugin/postgresql/PostgreSqlClient.java
Outdated
Show resolved
Hide resolved
| private static ColumnMapping charColumnMapping(int charLength) | ||
| { | ||
| if (charLength > CharType.MAX_LENGTH) { | ||
| return varcharColumnMapping(charLength); |
There was a problem hiding this comment.
Does this still preserve the char semantics of padding values?
There should be a test.
There was a problem hiding this comment.
good catch. it doesn't correctly pad on write path and during predicate pushdown (this may or may not be relevant, depending on remote's coercions, need checking)
it's pre-existing behavior though and applies to other connectors as well, see io.trino.plugin.jdbc.StandardColumnMappings#defaultCharColumnMapping
do you want to file an issue for this?
There was a problem hiding this comment.
#7152 - the title could be better though.
findepi
force-pushed
the
findepi/disable-incorrect-character-pushdown-in-postgresql-eaa946
branch
from
36f2403
to
dca8321
Compare
|
CI #5758 and some legit stuff |
findepi
force-pushed
the
findepi/disable-incorrect-character-pushdown-in-postgresql-eaa946
branch
from
dca8321
to
514272e
Compare
|
|
||
| if (domain.isOnlyNull() || | ||
| // PostgreSQL is case sensitive by default | ||
| domain.getValues().isDiscreteSet()) { |
There was a problem hiding this comment.
I'd put these two conditions on one line, with the comment inside the if block.
Maybe also add a bit more to the comment explaining how isDiscreteSet is related to case sensitivity. The comment doesn't really clarify much right now. (Ideally, I think CASE_INSENSITIVE_CHARACTER_PUSHDOWN would have a more detailed explanation of how that works, and then we could reference it here.)
There was a problem hiding this comment.
I put the comment in between so it unambiguous what it refers to
point take on the issue how discrete sets translate to actual predicates, and indeed it applies to CASE_INSENSITIVE_CHARACTER_PUSHDOWN
findepi
deleted the
findepi/disable-incorrect-character-pushdown-in-postgresql-eaa946
branch
Extracted from #6874