pgspider / sqlite_fdw

SQLite Foreign Data Wrapper for PostgreSQL
Other
219 stars 37 forks source link

Add human readable error message #75

Closed mkgrgis closed 1 year ago

mkgrgis commented 1 year ago

As separate function for a disallowed SQLite affinity with a PostgreSQL column data type.

nxhai98 commented 1 year ago

I run test for your branch in all versions and met a problem: sqlite_fdw cannot built on postgres 11.17 due to NullableDatum not define on PG11.17

error: unknown type name ‘NullableDatum’

Could you fix it?

Note: I tried to fix this issue by define NullableDatum by copying the definition in postgres.h of PG15 in case of PG11 in sqlite_fdw.h and this work:

#if PG_VERSION_NUM < 120000
/* NullableDatum is introduced from PG12, we define it here in case of PG11 or earlier. */
typedef struct NullableDatum
{
#define FIELDNO_NULLABLE_DATUM_DATUM 0
    Datum        value;
#define FIELDNO_NULLABLE_DATUM_ISNULL 1
    bool        isnull;
    /* due to alignment padding this could be used for flags for free */
} NullableDatum;
#endif
mkgrgis commented 1 year ago

Thanks for note about PG 11, @nxhai98 ! Fixed in https://github.com/pgspider/sqlite_fdw/pull/75/commits/5570b1213c1627f994a932cb44e4e89d8ff2a33b, please check.

nxhai98 commented 1 year ago

Thanks for note about PG 11, @nxhai98 ! Fixed in 5570b12, please check.

Thank for fixing, I confirmed the change.

I run test again and found the failed case: https://github.com/pgspider/sqlite_fdw/pull/75#discussion_r1205212910 Could you recheck that.

nxhai98 commented 1 year ago

@mkgrgis I completed my review. Thank for your support.

mkgrgis commented 1 year ago

Thanks for your reviews, @nxhai98 ! I am waiting on PR merge.

t-kataym commented 1 year ago

@mkgrgis , @nxhai98 Thank you for your contribution. I will merge the PR.