Skip to content

Conversation

@kenjis
Copy link
Member

@kenjis kenjis commented Aug 25, 2022

Ref #331

  • fixes for other databases.

Support:

  • OCi8
  • MySQLi
  • Postgre
  • SQLSRV

@kenjis kenjis added the enhancement New feature or request label Aug 25, 2022
@kenjis kenjis changed the title feat: fixes for other databases feat: support other databases Aug 25, 2022
$this->disableDBDebug();

if ($this->db->getPlatform() === 'OCI8' && $identifier === '') {
$identifier = ' ';
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

... and this?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah I see the whole commit message now, that explains this one. Seems like this is probably a bug in the framework driver itself, right?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure.
Oracle sees '' as NULL.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oracle Database currently treats a character value with a length of zero as null. However, this may not continue to be true in future releases, and Oracle recommends that you do not treat empty strings the same as nulls.

More see.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@datamweb I knew it, but I don't know what should we do in this case.
We try to insert '', but Oracle treats it as NULL and says "cannot insert NULL"

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use an alternate representation, such as a single space character, for an empty string.

It is what I did here.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ytetsuro have any recommendations? Or @sclubricants?

kenjis added 4 commits August 30, 2022 09:41
Query error: 1400, ORA-01400: cannot insert NULL into ("ORACLE"."db_auth_logins"."identifier"), query: INSERT INTO "db_auth_logins" ("ip_address", "user_agent", "id_type", "identifier", "user_id", "date", "success") VALUES ('0.0.0.0', '', 'email_password', '', NULL, '2022-08-24 22:10:33', 0)
@kenjis kenjis force-pushed the support-other-databases branch from 7085ad3 to 75c37d0 Compare August 30, 2022 00:41
@kenjis
Copy link
Member Author

kenjis commented Aug 30, 2022

I would like to merge this. Can someone approve?

If a true Oracle user knows better solution, anytime they can send a PR.

Copy link
Collaborator

@datamweb datamweb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!
See what @MGatner finally says.

Copy link
Member

@MGatner MGatner left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I pinged the only Oracle users I know (at least that I know that I know). This seems like a bigger question than this PR, but the code addresses the problem at hand so let's move forward.

@datamweb datamweb merged commit 1ef3977 into codeigniter4:develop Aug 30, 2022
@kenjis kenjis deleted the support-other-databases branch August 30, 2022 11:42
@MGatner
Copy link
Member

MGatner commented Aug 31, 2022

🥳

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants