Skip to content
This repository was archived by the owner on Jan 29, 2020. It is now read-only.

Short named parameters #305

Closed

Conversation

hamilok
Copy link
Contributor

@hamilok hamilok commented Feb 16, 2018

Fixed bug "ORA-00972: identifier is too long"

@hamilok
Copy link
Contributor Author

hamilok commented Feb 16, 2018

Before:

SELECT
  COUNT(1) AS "C"
FROM (
  SELECT
    "SYS_USER"."ID" AS "ID",
    "SYS_USER"."EMAIL" AS "EMAIL",
    "SYS_USER"."STATUS" AS "STATUS",
    TO_CHAR(LAST_SIGNIN_TIME, :subselect1subselect1LAST_SIGNIN_TIME1) AS "LAST_SIGNIN_TIME",
    TO_CHAR(CREATED_TIME, :subselect1subselect1CREATED_TIME1) AS "CREATED_TIME"
  FROM
    "SYS_USER"
  WHERE
    "LAST_SIGNIN_TIME" > SYSDATE - NUMTODSINTERVAL(:subselect1subselect1where1subpart1, :subselect1subselect1where1subpart2)
    AND
    "CREATED_TIME" > SYSDATE - NUMTODSINTERVAL(:subselect1subselect1where1subpart3, :subselect1subselect1where1subpart4
  )) "original_select"

After:

SELECT
  COUNT(1) AS "C"
FROM (
  SELECT
    "SYS_USER"."ID" AS "ID",
    "SYS_USER"."EMAIL" AS "EMAIL",
    "SYS_USER"."STATUS" AS "STATUS",
    TO_CHAR(LAST_SIGNIN_TIME, :ss1ss1LAST_SIGNIN_TIME1) AS "LAST_SIGNIN_TIME",
    TO_CHAR(CREATED_TIME, :ss1ss1CREATED_TIME1) AS "CREATED_TIME"
  FROM
    "SYS_USER"
  WHERE
    "LAST_SIGNIN_TIME" > SYSDATE - NUMTODSINTERVAL(:ss1ss1where1sp1, :ss1ss1where1sp2)
    AND
    "CREATED_TIME" > SYSDATE - NUMTODSINTERVAL(:ss1ss1where1sp3, :ss1ss1where1sp4
  )) "original_select"

@michalbundyra
Copy link
Member

@hamilok Please check travis build:
https://travis-ci.org/zendframework/zend-db/builds/342271399?utm_source=github_status&utm_medium=notification

It's failing because of your change. Could you please fix them and add your test case?

@hamilok hamilok changed the title short named parameters Short named parameters Feb 16, 2018
@hamilok
Copy link
Contributor Author

hamilok commented Feb 23, 2018

@Ocramius please, merge it

@Ocramius
Copy link
Member

This seems to patch just the prefix/suffix to shorten it, but adding multiple sub-selects will lead to the same issue anyway...

@hamilok
Copy link
Contributor Author

hamilok commented Feb 23, 2018

Very likely, but now even with one "subselect" there is a problem, and this patch corrects it.
@Ocramius What do you think?

@ezimuel
Copy link
Contributor

ezimuel commented Apr 3, 2018

@Ocramius are you sure that this doesn't fix also multiple sub-selects use cases?

@Ocramius
Copy link
Member

Ocramius commented Apr 3, 2018

@ezimuel yes: this is basically "let's fix an overflow by making the payload size smaller", instead of fixing the actual overflow.

@ezimuel
Copy link
Contributor

ezimuel commented Apr 3, 2018

@hamilok you mentioned bug ORA-00972, can you add more information on this? I don't see any issue opened with this reference. Thanks!

@ezimuel
Copy link
Contributor

ezimuel commented Apr 3, 2018

@Ocramius I see, anyway it's still a fix to reduce the issue. A better solution will be to rewrite the string parameters using some internal table reference, as I did here for #304.

@Ocramius
Copy link
Member

Ocramius commented Apr 3, 2018

anyway it's still a fix to reduce the issue

I'm extremely vary of anything that doesn't fix the issue at the source, so I can't agree with going with the current solution. The approach taken in #304 looks more appropriate.

@ezimuel
Copy link
Contributor

ezimuel commented Apr 3, 2018

@Ocramius you right, I'm closing this in favor of alternative solution based on #304 approach. @hamilok can you have a look at this solution and propose a new PR? Thanks anyway for your contribution here!

@ezimuel ezimuel closed this Apr 3, 2018
@hamilok hamilok deleted the feature/short-named-parameters branch April 12, 2018 10:36
@hamilok
Copy link
Contributor Author

hamilok commented Apr 12, 2018

@ezimuel solution based on #304 approach don't fix ORA-00972 problem.

What was in my changes?

The first type of placeholder is based on the name of the column:
: subselect1subselect1LAST_SIGNIN_TIME1

16 user characters and 21 system characters. This is a clear overflow.
It is necessary to reduce to a minimum of 5 system characters. Other characters for the user.
: s1s1LAST_SIGNIN_TIME1

The second placeholder is auto-generated:
: subselect1subselect1where1subpart1

Minimize up to 12 characters -: s1s1where1s1
Or up to 8 characters -: s1s1w1s1

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants