Skip to content
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

Long view/table names causing issues with Oracle DB:s < version 12.1 #171

Closed
callebstrom opened this issue Dec 14, 2017 · 5 comments
Closed

Comments

@callebstrom
Copy link
Contributor

callebstrom commented Dec 14, 2017

Hi!

When running this application using the oci8 Sequel adapter against a Oracle DB (that is running version 12.1 or lower) I encounter the ORA-00972 error (identifier too long).

Turns out there is a limitation on the length of identifiers (view names, table names, column names) that does not allow names longer than 30 characters (iso-8859-1 characters).

I provisioned a PostgreSQL db and extracted all table and view names that are longer than 30 characters, see below:

latest_pact_publications_by_consumer_versions
latest_pact_publication_revision_numbers
latest_verification_id_for_consumer_version_and_provider_version
latest_pact_publications_by_consumer_versions
latest_pact_consumer_version_orders
latest_tagged_pact_consumer_version_orders
latest_tagged_pact_consumer_version_orders
latest_verification_id_for_consumer_version_and_provider_version
latest_tagged_pact_publications
latest_tagged_pact_publications
latest_tagged_pact_publications
latest_pact_publication_revision_numbers
latest_pact_publications_by_consumer_versions
latest_pact_consumer_version_orders
latest_tagged_pact_consumer_version_orders
latest_verification_id_for_consumer_version_and_provider_version

Would it be possible to come up with some new, shorter, names for these views and tables in order to ensure compability with Oracle DB 12.1 and below?

It might also be worth noting that PostgreSQL truncates these names as well, even thought they have a less strict naming policy.

I would more than gladely implement this myself and submit a PR, but I would prefer a go-ahead fro you guys first to ensure I'm not implementing anything that would be thrown away.

I'm thinking of just replacing "latest" with "l", "tagged" with "t", "consumer version" with "cv".

Wdyt?

Best regards,
Calle

@bethesque
Copy link
Member

Thanks for your offer Calle. To be honest, every new database that we officially support is a massive pain. Just trying to keep the functionality compatible between Postgres and MySQL is already a time sync. On top of that, doing table renames in a backwards compatible way would be very tricky. The only way to be able to run all the past migrations on Oracle would be to change the existing migrations, and we can't go back and change the existing migrations because this will break all the current deployments. I don't see how it can be done without writing some very hacky code which I'm not keen on supporting for the rest of the project's lifespan! Do you have an elegant solution in mind?

The only other option I can think of is showing you how to maintain your own fork (currently some class names map directly to the table names, and if the table names were changed, we'd need to explicitly configure that, but other than that it would be straightforward)

@callebstrom
Copy link
Contributor Author

@bethesque Thanks for your feedback!

Those are some valid points, I do imagine pgsql and mysql are enough pain as it is...

The most obvious solution from my end would be to actually upgrade to a later Oracle database version however, I suspect this would also be a massive pain unfortunately (due to legacy concerns etc).

I'm not too familiar with the pact_broker code base (or ruby for that matter), but is there a way of hooking into all Sequel calls and actually rewriting the target view or table name?

Perhaps this could be an optional concept that is made available as part of the application configuration? Say an additional configuration key that would look something along these lines (sorry for any potential horrible/invalid syntax):

app = PactBroker::App.new do | config |
  config.logger = ::Logger.new($stdout)
  config.logger.level = Logger::WARN
  config.database_connection = Sequel.connect(DATABASE_CREDENTIALS.merge(logger: DatabaseLogger.new(config.logger), encoding: 'utf8'))
  config.database_connection.timezone = :utc
  config.database_rewrites = { 
    "views" => {
      "latest_pact_publications_by_consumer_versions" => "l_pact_publications_by_cv",
    }
  }
end

I think the key here is making these hacks optional and also avoid you guys having to own them.

I would be up for maintaining a fork but only as a last resort to be honest.

@bethesque
Copy link
Member

I've had a poke around in the Sequel gem to see if I could do what you are suggesting. I found a (very hacky) way to modify some, but not all of the SQL, and we'd need ALL of the sql to be consistently modified for this to work.

Have you considered using one of the hosted brokers here to avoid having to maintain your own instance? https://pact.dius.com.au/

@callebstrom
Copy link
Contributor Author

@bethesque I didn't realize that there was a hosted solution actually, so that's great. However, I am afraid due to privacy concerns we would have to host our own.

If the solution is very hacky it might be worthwhile for us to just upgrade our database in order to support the table name lengths, even though that will probably quite a pain.

Big thanks for looking into this though!

@bethesque
Copy link
Member

No worries.

The privacy concern is something that a few people have mentioned as a reason for hosting their own brokers, and we're interested in exploring people's concerns to see if there are ways we can mitigate them. Given that there should be no real data used in the interactions, can you articulate the nature of the concerns more specifically? Feel free to send me an email at bskurrie@dius.com.au if you'd like to discuss it more privately.

YOU54F pushed a commit to YOU54F/pact_broker that referenced this issue Jul 31, 2024
* feat: adding support for share success and failure actions

* Merging latest 'main' changes in to Action Object Extensions branch (pact-foundation#170)

* chore: suggest multilevel parameters (pact-foundation#155)

* chore: suggest multilevel parameters

* Update versions/1.0.0.md

Co-authored-by: Frank Kilcommins <fkilcommins@gmail.com>

* Update examples/1.0.0/ExtendedParametersExample.workflow.yaml

Co-authored-by: Frank Kilcommins <fkilcommins@gmail.com>

* Update versions/1.0.0.md

Co-authored-by: Frank Kilcommins <fkilcommins@gmail.com>

---------

Co-authored-by: Frank Kilcommins <fkilcommins@gmail.com>

* Remove references to WHATWG to avoid confusion (pact-foundation#145)

* Remove references to WHATWG to avoid confusion

* Correct relative reference wording

* Section 4.2 not 4.1!

* Simplify URI wording

* Adjust Step Parameters desc to cater for Workflow parameters addition (pact-foundation#169)

* feat: add `dependsOn` capability for workflow level (pact-foundation#164)

* feat: add `dependsOn` capability for workflow level

* chore: typo fix

* chore: grammer fix

* feat: Add Request Body Object (pact-foundation#162)

* feat: Add Request Body Object

* chore: fix typos in examples

* Clarity on referencing Components Parameters (pact-foundation#149)

* Clarity on referencing Components Parameters

* Remove Reference Object to avoid clash with JSON Schema keyword. Replace with expression based referencing

* chore: keep fixed field link names consistent

* chore: Name component parameters as type Reusable Parameter Object

* chore: adjust Workflow level parameters to use Reusable Parameter Objects

---------

Co-authored-by: Dmytro Anansky <dmytro@redocly.com>
Co-authored-by: Nick Denny <nick@apimetrics.com>

* Add Reusable Object and referencing ability

* chore: fix yaml example indentation

* chore: fix JSON examples

* chore: fix typo in `workflowStepActions`

---------

Co-authored-by: Dmytro Anansky <dmytro@redocly.com>
Co-authored-by: Nick Denny <nick@apimetrics.com>
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

No branches or pull requests

2 participants