ibarwick / firebird_fdw

A PostgreSQL foreign data wrapper (FDW) for Firebird - latest version 1.4.0 (2024-05-11)
https://sql-info.de/postgresql/firebird-fdw/index.html
Other
36 stars 9 forks source link

Bool WHERE escalation in Firebird smallint #9

Closed mkgrgis closed 3 years ago

mkgrgis commented 5 years ago

Firebird (2.5, Linux)

CREATE TABLE "fb_t" ("i" SMALLINT, "n" DOUBLE PRECISION);
INSERT INTO "fb_t" ("i", "n") VALUES (0, 0.45454);
INSERT INTO "fb_t" ("i", "n") VALUES (1, 3.1415296);
INSERT INTO "fb_t" ("i", "n") VALUES (0, 2.7182818);

De facto smallint i={0|1}, pseudo-bool;

Postgres (12, Linux)

CREATE EXTENSION firebird_fdw;
CREATE FOREIGN DATA WRAPPER firebird  HANDLER firebird_fdw_handler  VALIDATOR firebird_fdw_validator;

CREATE SERVER firebird_server
  FOREIGN DATA WRAPPER firebird
  OPTIONS <...>

CREATE USER MAPPING <...>

CREATE FOREIGN TABLE "fb_probe"(
  "i" BOOL OPTIONS (column_name 'i'),
  "n" DOUBLE OPTIONS (column_name 'n')
)
SERVER firebird_server
OPTIONS(
  table_name 'fb_t'
);

SELECT * FROM "fb_probe" WHERE "i"; => ERROR!

SELECT * FROM "fb_probe" WHERE NOT "i"; => ERROR!

mkgrgis commented 5 years ago

Text for the situation from psql.

Unhandled XACT_EVENT_PARALLEL_* event
Unable to execute remote query
ibarwick commented 5 years ago

Hi

Neither Firebird nor PostgreSQL actually support this syntax.

PostgreSQL:

postgres=# \d implied_bool_test
                Table "public.implied_bool_test"
 Column |         Type          | Collation | Nullable | Default
--------+-----------------------+-----------+----------+---------
 i      | smallint              |           |          |
 val    | character varying(10) |           |          |

postgres=# SELECT * FROM implied_bool_test WHERE i;
ERROR:  argument of WHERE must be type boolean, not type smallint
LINE 1: SELECT * FROM implied_bool_test WHERE i;

Firebird 3.0:

SQL> SHOW TABLE fb_t;
I                               SMALLINT Nullable
N                               DOUBLE PRECISION Nullable
SQL> SELECT * FROM fb_t WHERE i;
Statement failed, SQLSTATE = 22000
Dynamic SQL Error
-SQL error code = -104
-Invalid usage of boolean expression

Firebird 2.5 errors out with ERROR: Unexpected end of command.

I don't think it is desirable or practical to implement this.

Either upgrade to Firebird 3 or use WHERE i = 0 / WHERE i != 0.

BTW you don't need to explicitly quote object names unless you want them in a specific case on both nodes.

mkgrgis commented 5 years ago

Ok. You have demonstrate the situation. But it is not problematic situation. Your pg object element is I SMALLINT Nullable, but my pg object element was "i" BOOL OPTIONS (column_name 'i'), If your pg object element is smallint, of course, it is not predicate for "WHERE". But i haven't use the smallint element as prediacate anywhere, neither in Firebird nor in PostgreSQL. My attribute in Postgres relation (in the "CREATE FOREIGN TABLE") was BOOL.

If i have a smallint Firebird as base for bool Postgres, your program successful indicate logic (bool) content. But it's only right indication. There is no right escalation Postgres BOOL condition to firebird smallint =0.

P. S. "explicitly quote object names" used because there is much Korean and other unicode names of objects - tables, views, triggers etc. And than in Postgres default register is downcase, but in Firebird uppercase. It seems, Firebird 3 supports unicode names of objects too.

mkgrgis commented 5 years ago

Is there any diagnostics for my case, @ibarwick ? I can't patch, because I don't know what is XACT_EVENT_PARALLEL. But I think type from PostgreSQL and type from Firebird meets only in Your program firebird_fdw. How can I help?

mkgrgis commented 3 years ago

I think this issue is about behaviour of https://github.com/ibarwick/firebird_fdw/blob/master/src/convert.c convertDatum(Datum datum, Oid type) if boolfrom Postgres have smallint as base in Firebird.

ibarwick commented 3 years ago

Looking at this again as I was reviewing boolean handling.

So, what you are requesting is to map the PostgreSQL boolean type defined in a foreign table to an arbitrary Firebird datatype - you suggest:

De facto smallint i={0|1}, pseudo-bool;

That is certainly not going to happen, as it would be hard-coding a particular way of converting pseudo-booleans; from previous experience with databases where there is no true boolean type available, there is more than one way of doing this (e.g. CHAR(1) with t/f, or by creating a DOMAIN), and it would be unacceptable to hard-code a particular variant. It would also break support for Firebird 3 booleans.

Theoretically it would be possible to add a column level option which defines a flexible mapping along the lines of:

CREATE FOREIGN TABLE booltest (
  ...
  boolval BOOL OPTIONS (remote_datatype 'smallint', true_val '1', false_val '0'),
)
...

However that would add substantial complexity to the code, as every code path where PostgreSQL expects to handle a boolean (more places then you link to above) would have to check for the presence of such options and handle them accordingly. It might be doable, but at this point I am not able to consider it due to lack of time.

Moreover there is a straightforward workaround which is usable now and can be implemented with any pseudo-bool type simply by creating an appropriate view over the foreign table, e.g.:

    CREATE TABLE booltest (
      id INT,
      boolval SMALLINT
    );
    INSERT INTO booltest VALUES(1, 1);
    INSERT INTO booltest VALUES(2, 0);
    CREATE FOREIGN TABLE fb_booltest (
      id INT,
      boolval SMALLINT
    )
    SERVER fb_test
    OPTIONS(
      table_name 'booltest'
    );

    CREATE VIEW fb_booltest_v AS
      SELECT id,
             CASE
               WHEN boolval = 0 THEN false
               ELSE true
             END
               AS boolval
        FROM fb_booltest;

You can then use boolean conditions etc. as desired:

postgres=# SELECT * FROM fb_booltest_v WHERE boolval;
 id | boolval
----+---------
  1 | t
(1 row)

postgres=# SELECT * FROM fb_booltest_v WHERE boolval IS FALSE;
 id | boolval
----+---------
  2 | f
(1 row)
mkgrgis commented 3 years ago

Thank You, @ibarwick. The view works. Is there a pushing of WHERE condition to Firebird side for the view? It seems to me by Your code and my tests, that there is no pushing and there is FULL SCAN for the table on Firebird side. Is it really? I know, that there is a difference between IS NOT TRUE and IS FALSE conditions in Postgresql and Firebird.

ibarwick commented 3 years ago

Correct, no pushdown of the view:

postgres=# explain SELECT * FROM fb_booltest where boolval = 1;
                                QUERY PLAN                                
--------------------------------------------------------------------------
 Foreign Scan on fb_booltest  (cost=10.00..12.00 rows=2 width=6)
   Firebird query: SELECT id, boolval FROM booltest WHERE ((boolval = 1))
(2 rows)

postgres=# explain SELECT * FROM fb_booltest_v where boolval is true;
                              QUERY PLAN                              
----------------------------------------------------------------------
 Foreign Scan on fb_booltest  (cost=10.00..12.01 rows=2 width=5)
   Filter: (CASE WHEN (boolval = 0) THEN false ELSE true END IS TRUE)
   Firebird query: SELECT id, boolval FROM booltest
(3 rows)

Not sure at the moment whether it's feasible to push the CASE statement to Firebird (though without a boolean data type in Firebird it's probably a moot point anyway). I'm currently (among other things) trying to expand the range of items which can be pushed down (from 1.2.0 that will include booleans, but only FB 3+ of course), but that will take some time to implement.

mkgrgis commented 3 years ago

That is certainly not going to happen, as it would be hard-coding a particular way of converting pseudo-booleans;

There is some official casts (not "particular way of converting") for boolean from PostgreSQL's documentation. The cast from integer types is base of pushing for internal queries. I think the int/smallint/bigint cast to boolean can be hard-coded. https://www.postgresql.jp/document/12/html/datatype-boolean.html https://postgrespro.ru/docs/postgresql/12/datatype-boolean#

The boolval BOOL OPTIONS (remote_datatype 'smallint', true_val '1', false_val '0') style can be useful only for string types. We can not automate pushing bool WHERE for Firebird string datatypes because there are 3 official string constants for true and false. So, c1 OR c2 OR c3 is not good idea for pushing to Firebird.

ibarwick commented 3 years ago

I think the int/smallint/bigint cast to boolean can be hard-coded.

Looking at other FDWs, I see at least oracle_fdw has support for mapping boolean columns to Oracle numeric columns as implicit booleans, and postgres_fdw will at least read integer columns containing 0/1 as booleans (though that's more accidental), so there is some precedent for doing that. I had some unexpected free time recently and while implementing something else it occurred to me it might actually be possible to implement this for Firebird without breaking support for actual Firebird boolean columns, which turned out to be an interesting albeit non-trivial intellectual exercise. See commit cc29bb8b87bb839cd438761689c42f54dbb7c920 for details; note this is experimental.

mkgrgis commented 3 years ago

Thanks, @ibarwick ! The realisation is great! I'll test for

unanticipated query tree variants

Usual case of usage of Pg's bool to Firebird pushing is simple queries both for students and industrial db access.

mkgrgis commented 3 years ago

The test has been passed! Congratulations! My test calls WHERE pushing for different integer datatypes and combines implicit bool with real Firebird 3.0+ bool. No problems! So, "unanticipated query tree variants" is possible, but it will be very rare.

Firebird:

CREATE TABLE "fb_bool"
(
  "id" integer NOT NULL,
  "si" smallint,
  "i"  smallint,
  "bi" bigint,
  "b"  boolean
);

INSERT INTO "fb_bool" ("id","si", "i", "bi", "b") VALUES (1, 0, NULL, 1, true);
INSERT INTO "fb_bool" ("id","si", "i", "bi", "b") VALUES (2, 1, 0, 2, true);
INSERT INTO "fb_bool" ("id","si", "i", "bi", "b") VALUES (3, 2, 1, NULL, true);
INSERT INTO "fb_bool" ("id","si", "i", "bi", "b") VALUES (4, NULL, 2, 0, true);
INSERT INTO "fb_bool" ("id","si", "i", "bi", "b") VALUES (5, 0, NULL, 1, false);
INSERT INTO "fb_bool" ("id","si", "i", "bi", "b") VALUES (6, 1, 0, 2, false);
INSERT INTO "fb_bool" ("id","si", "i", "bi", "b") VALUES (7, 2, 1, NULL, false);
INSERT INTO "fb_bool" ("id","si", "i", "bi", "b") VALUES (8, NULL, 2, 0, false);
INSERT INTO "fb_bool" ("id","si", "i", "bi", "b") VALUES (9, 0, NULL, 1, NULL);
INSERT INTO "fb_bool" ("id","si", "i", "bi", "b") VALUES (10, 1, 0, 2, NULL);
INSERT INTO "fb_bool" ("id","si", "i", "bi", "b") VALUES (11, 2, 1, NULL, NULL);
INSERT INTO "fb_bool" ("id","si", "i", "bi", "b") VALUES (12, NULL, 2, 0, NULL);
COMMIT;

PostgreSQL:

CREATE FOREIGN TABLE "pg_bool" (
  "id" integer NOT NULL,
  "si" bool OPTIONS (implicit_bool_type 'true'),
  "i"  bool OPTIONS (implicit_bool_type 'true'),
  "bi" bool OPTIONS (implicit_bool_type 'true'),
  "b"  bool
)
 SERVER "fb_Test"
OPTIONS ( table_name 'fb_bool');

SELECT * FROM "pg_bool";

SELECT * FROM "pg_bool" WHERE "si";
SELECT * FROM "pg_bool" WHERE "i";
SELECT * FROM "pg_bool" WHERE "bi";
SELECT * FROM "pg_bool" WHERE "b";

SELECT * FROM "pg_bool" WHERE NOT "si";
SELECT * FROM "pg_bool" WHERE NOT "i";
SELECT * FROM "pg_bool" WHERE NOT "bi";
SELECT * FROM "pg_bool" WHERE NOT "b";

SELECT * FROM "pg_bool" WHERE "si" IS NULL;
SELECT * FROM "pg_bool" WHERE "i" IS NULL;
SELECT * FROM "pg_bool" WHERE "bi" IS NULL;
SELECT * FROM "pg_bool" WHERE "b" IS NULL;

SELECT * FROM "pg_bool" WHERE NOT "si" AND NOT "b";
SELECT * FROM "pg_bool" WHERE "bi" AND NOT "b";
SELECT * FROM "pg_bool" WHERE "bi" AND "b";
SELECT * FROM "pg_bool" WHERE NOT "bi" AND "b";
SELECT * FROM "pg_bool" WHERE "si" AND "b" IS NULL;