sqlite_fdw
sqlite_fdw copied to clipboard
pgspider
RETURNING support
[feature request]
SQL RETURNING have been introduced in SQLite 3.35.0.
SQLite's syntax for RETURNING is modelled after PostgreSQL.
Hence look like there is no special behaviour for RETURNING in SQLite not mapped to PostgreSQL RETURNING behavoiur.
Also added new UPSERT syntax, see https://sqlite.org/releaselog/3_35_0.html
During drafting I have seen, we need to increase SQLite version for RETURNING support, 3.34.0 now.
@t-kataym , if a new release is yet planned, I am ready to present RETURNING C infrastructure aligned with the newest postgres_fdw or even initial INSERT ... RETURNING and UPDATE ... RETURNING support. Please refer my special branch.
@mkgrgis Thank you for your effort. We are in progress to release new version of sqlite_fdw supporting PostgreSQL 17.0. We would like to merge new feature after this release. You will need to rebase the code. I'm sorry for bothering you.
Thanks for clarification, @t-kataym! I am ready to rebase existed PRs.
What about testing of 2-3 obsolete PostgreSQL versions until this doesn't make a C code harder? As we can see in oracle_fdw documantation, the author doesn't exclude old pg versions from tests if this need no hard code changes. Look like he supports old pg versions until C code for new versions is formally compatible.
We support 5 major versions of PostgreSQL .
When releasing or developing new feature, we are adding and executing new tests for all old versions which we are supporting. In order to save a cost/effort, we don't support obsolete PostgreSQL versions. And tests for unsupported versions are removed.
In order to save a cost/effort, we don't support obsolete PostgreSQL versions.
Author of oracle_fdw also don't support obsolete PostgreSQL versions, but leaves tests until introducing of some code innovations which is not compatible with old pg versions.
And tests for unsupported versions are removed.
Why the policy "remove only if new code breaks some very old tests" is not better? Author of oracle_fdw also save a cost/effort, but does this. He thinks popular FDW sometimes will compiled with unsupported PostgreSQL versions. Also this code compatibility with old pg versions sometimes is useful for academical researches about PostgreSQL code. How do you think, @t-kataym ?
Do you mean to keep test code despite that they are not executed for the revision of source code? I want to keep only valid/maintained tests ("valid/maintained" means that we executed tests and confirmed test results.) Tests for old version can be acquired from old version of sqlite_fdw.
(Please correct me if I'm misunderstanding)
Do you mean to keep test code despite that they are not executed for the revision of source code?
No. Author of Oracle FDW removes tests for unsupported pg versions only if adopting of a new feature/commit to old pg versions takes unwanted cost/effort. Otherwise all tests leaves and modified as the newest one.
For example, my UUID support is compatible with PostgreSQL down to 9.6 without any refactoring. So, it was no cost/effort to support old versions. But RETURNING support in my draft is based on modern FDW interface and compatible with pg 11+. This means UUID support PR is not reason to remove old tests, but RETURNING support is. What about such policy of maximal compatibility without increasing costs or efforts, @t-kataym ?
Judging whether we keep tests or not requires an effort. We simply keep only tests of supporting pg version. (Seen from one FDW, its may be a small effort. But we maintain a lot of FWDs including un-published FDWs in our company. We need to apply the same policy for all FDWs.) I hope you understand.
I hope you understand.
Yes. This is understandable unified internal procedure. Thanks for clarification, @t-kataym !
@t-kataym , could you please explain me better PR order for pgspider team? Now I have in my repo
- GIS PR with some encapsulated error outside of my code,
- tested
RETURNINGimplementation - tested attached schemas with
PRAGMAs support.
Which of possible review plans will better for pgspider team? For me all of this possible PRs is simple for rebasing to any other.
@mkgrgis Thank you for your proposal.
We would appreciate it if you could carry out the following in order of priority.
- tested RETURNING implementation
- tested attached schemas with PRAGMAs support.
- GIS PR with some encapsulated error outside of my code,
We would appreciate it if you could carry out the following in order of priority.
1. tested RETURNING implementation
I am ready for https://github.com/pgspider/sqlite_fdw/pull/107 review by pgspider team, @t-kataym .
@t-kataym , now I have
- Rebased after GIS support
RETURNINGsupport PR - Simple JSON operators PR https://github.com/pgspider/sqlite_fdw/pull/109
- Schemas (attached DBs) support
Will 2->1->3 order of reviews good for pgspider team? Could you share your opinion?
@mkgrgis I'm sorry for the late reply.
Will 2->1->3 order of reviews good for pgspider team? Could you share your opinion?
It's okay for us. If 1 is not proceeded smoothly, it is no problem to do 3 (2 ->3->1).
Thanks, @t-kataym , I am ready for JSON PR review, than I'll create schemas/PRAGMAs PR, but in parallel I am ready for continue RETURNING diagnostics and consultations. Please call pgspider team members for review and diagnostics.
it is no problem to do 3 (2 ->3->1).
@t-kataym , I am sorry for delay, but JSON CI problem is still not diagnosed by pgspider team. Could you please write approximate date of review? Also I am ready for parallel work on existed PR about inet data type before schemas support. Can pgspider team also review this new PR?
@t-kataym , I have fixed some CI problems in https://github.com/pgspider/sqlite_fdw/pull/109 , but SQLite install in CI environment is absolutely not effective, all proofs and comments are by the link. Could you please call CI engineers for fixing and ask @lamdn1409 about C code review?