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

Postgres explain plan is not correctly optimezed if the data is a "double" but the collumn is a "numeric" #3285

Closed
wants to merge 1 commit into from

Conversation

CharlesLgn
Copy link

@CharlesLgn CharlesLgn commented Jun 14, 2024

In Postgres, if we have a collumn stored as a "numeric", but store "double in it", the execution plan will not be optimized.

Driver Version

postgresql-42.7.3

Java Version

JDK 21

To Reproduce

  • Create a database with numeric column
create table INVOICE (
    id varchar(25) not null constraint pk_invoice primary key,
    amount numeric,
    contract_id varchar(25) not null
);

create table CONTRACT (
    id varchar(25) not null constraint pk_contract primary key,
    reference varchar(25)
);
  • use the request : select id from invoice i join contract c on c.id = i.contract_id where i.amount between ? and ?

When we use a PreparedStatement, the system should use an implicite cast as a numeric to calculate the execution plan, but it used ('the value')::double precision, so the execution plan could be false leading to performance issue (in my case, we have 8 million + invoice in the database, and the prepared request take 3 sec to execute)

The problème is generate by this change : 06abfb7

Now, the executed request for data -10 and 3.4 will be select id from invoice i join contract c on c.id = i.contract_id where i.amount between ('-10'::double precision) and ('3.4'::double precision).

As I understand, this modification is due to a security problème with thing like -? that could generate son SQL injection if the value is a negative number.

Correction proposition

Instead of using a cast, only put the data between parenthesis for number, so we have -? becomming -(-2.548) istead of -('-2.548'::double precision)

Fixes #3284

@CharlesLgn CharlesLgn changed the title Postgres explain plan is not correctly optimezed if the data is a "double" but the collumn is a "numeric" #3284 Postgres explain plan is not correctly optimezed if the data is a "double" but the collumn is a "numeric" Jun 14, 2024
@vlsi
Copy link
Member

vlsi commented Jun 14, 2024

When we use a PreparedStatement, the system should use an implicite cast as a numeric to calculate the execution plan

Please clarify why do you think so.
When you use PreparedStatement, the driver passes the value according to the set... method.
For instance, the execution plan might differ if you use setDouble(1, ...) vs setBigDecimal(1, ..).

I think the PR is invalid, and we should rather add a documentation sample that highlights PostgreSQL behavior.

@CharlesLgn
Copy link
Author

CharlesLgn commented Jun 14, 2024

Please clarify why do you think so.

before this two changes : 06abfb7 ; 93b0fcb

The behavior was different regarding number object value in PreparedStatement in 01-2024.

but, to go further, here is my request (simplified regarding some company privacy):

SELECT  Facture.ID, Facture.REFERENCE, Facture.MONTANTTTC, (
  SELECT Acteur.NAME || ' ' || CASE WHEN Acteur.LASTNAME IS NULL THEN '' ELSE Acteur.LASTNAME END
  FROM Acteur
  WHERE Contrat.Acteur_ID = Acteur.ID), Contrat.REFERENCE, Offre.LIBELLE
FROM Facture
  join Contrat on Facture.CONTRAT_ID = Contrat.ID
  join Offre on Contrat.OFFREPRODUIT_ID = Offre.ID
where Facture.MONTANTTTC between ? and ?

This request took less than 1ms in the last pgjdbc version. now it took more than 3 seconds.

In the object, montantTTC is a double, so we use st.setDouble(....
Now we should use st.setBigDecimal([...], BigDecimal.valueOf(...)). We must do the same things in our batchs, leading to billion conversion object,

Moreover, if I use a query tool like DBeaver/DataGrip/..., this conversion does not have this contrainst to cast the value.

here are the explained plan :

@davecramer
Copy link
Member

Are you requesting SimpleQuery mode when executing this ?

@CharlesLgn
Copy link
Author

Are you requesting SimpleQuery mode when executing this ?

In fact I am not. so as I understand, and as @vlsi said in the issue post :

There's no way driver could do about it, and if it worked previously, it was a pure luck 🤷‍♂️

Sorry to have created a PR for no reason 😓

@CharlesLgn CharlesLgn closed this Jun 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Postgres explain plan is not correctly optimezed if the data is a "double" but the collumn is a "numeric"
3 participants