Skip to content

Fix/oracle last insert id #52416

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

Draft
wants to merge 19 commits into
base: master
Choose a base branch
from
Draft

Fix/oracle last insert id #52416

wants to merge 19 commits into from

Conversation

come-nc
Copy link
Contributor

@come-nc come-nc commented Apr 24, 2025

Same as #46304 but on current master

Checklist

come-nc and others added 4 commits April 24, 2025 14:12
Signed-off-by: Côme Chilliet <come.chilliet@nextcloud.com>
Signed-off-by: Joas Schilling <coding@schilljs.com>
Signed-off-by: Joas Schilling <coding@schilljs.com>
@come-nc come-nc self-assigned this Apr 24, 2025
Signed-off-by: Côme Chilliet <come.chilliet@nextcloud.com>
@come-nc come-nc force-pushed the fix/oracle-insert-id branch from 674d2d8 to 79325cb Compare April 24, 2025 13:04
come-nc added 14 commits April 24, 2025 15:26
Signed-off-by: Côme Chilliet <come.chilliet@nextcloud.com>
Signed-off-by: Côme Chilliet <come.chilliet@nextcloud.com>
Signed-off-by: Côme Chilliet <come.chilliet@nextcloud.com>
Signed-off-by: Côme Chilliet <come.chilliet@nextcloud.com>
Signed-off-by: Côme Chilliet <come.chilliet@nextcloud.com>
Signed-off-by: Côme Chilliet <come.chilliet@nextcloud.com>
Signed-off-by: Côme Chilliet <come.chilliet@nextcloud.com>
Signed-off-by: Côme Chilliet <come.chilliet@nextcloud.com>
Signed-off-by: Côme Chilliet <come.chilliet@nextcloud.com>
…nstructor

Signed-off-by: Côme Chilliet <come.chilliet@nextcloud.com>
Signed-off-by: Côme Chilliet <come.chilliet@nextcloud.com>
Signed-off-by: Côme Chilliet <come.chilliet@nextcloud.com>
Signed-off-by: Côme Chilliet <come.chilliet@nextcloud.com>
Signed-off-by: Côme Chilliet <come.chilliet@nextcloud.com>
@come-nc
Copy link
Contributor Author

come-nc commented Apr 24, 2025

Current error

string(179) "DECLARE vRowid NUMBER; BEGIN INSERT INTO INTO "oc_storages" ("id", "available") VALUES(:dcValue1, :dcValue2) RETURNING "numeric_id" INTO vRowid; dbms_output.put_line(vRowid); END;"
Doctrine\DBAL\Driver\OCI8\Exception\Error: ORA-06550: line 1, column 42:
PL/SQL: ORA-00903: invalid table name
ORA-06550: line 1, column 30:
PL/SQL: SQL Statement ignored

What I learned:

  • I think rowid will not be useful for us and we’ll need to know the name of the autoincrement column
  • There is a returning keyword in postgres which might be good to use as well if we go that route
  • It seems impossible to use returning into without a variable declared and a begin block
  • That said I failed to find the correct syntax for said begin block yet.
  • I hate Oracle.

[EDIT] ref: https://docs.oracle.com/en/database/oracle/oracle-database/19/lnpls/RETURNING-INTO-clause.html

@come-nc
Copy link
Contributor Author

come-nc commented Apr 28, 2025

So, my last error was from double INTO keyword.

That said, I tried a lot of stuff locally and cannot get this to work. The request pass but I do not get the id from it.

  • I tried switching to executeQuery to get the id in the result object, but the result object is empty/buggy
  • I tried switching vRowid to :vRowid to bind it as an output parameter but that’s not supported anymore

I think this is simply not possible (without terrible hacky stuff) with doctrine/dbal.
Related refs:

  1. Deprecate Statement::bindParam() doctrine/dbal#5563
  2. [Oracle] DBAL 4 Bind outputs for stored procedure doctrine/dbal#6782
  3. (OCI8 driver) The problem with getting the result of the procedure execution from the bound param doctrine/dbal#6106
  4. Identity columns doctrine/dbal#4744

At this point I suggest we open an issue on doctrine/dbal side asking for help and information.
I do not understand how autoincrement columns are usable with their current API.
Identity columns seems to be an alternative but not supported yet.

I do not think it makes sense for us to switch to uuids, to much work and changes for no benefits. Most mappers based on our current APIs rely on autoincrement ids.

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.

2 participants