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

fix(QueryBuilder): Restrict identifier length to 30 characters due to Oracle limitations #48361

Merged
merged 1 commit into from
Sep 26, 2024

Conversation

provokateurin
Copy link
Member

Summary

According to https://docs.nextcloud.com/server/latest/admin_manual/installation/system_requirements.html we still support Oracle versions <12.2.
https://docs.oracle.com/en/database/oracle/oracle-database/23/sqlrf/Database-Object-Names-and-Qualifiers.html says:
If COMPATIBLE is set to a value of 12.2 or higher, then names must be from 1 to 128 bytes long and If COMPATIBLE is set to a value lower than 12.2, then names must be from 1 to 30 bytes long.
This means we must prevent any identifier with more than 30 characters as long as we support Oracle <12.2.

This check should also be extended to table names and schema names, but I'm not sure how to do that best.
I found this due to https://github.com/nextcloud/groupfolders/actions/runs/10960223441/job/30434816204 (which tests against Oracle 11) which triggers this query:

 SELECT `g`.`folder_id`,
       `g`.`group_id`,
       `g`.`circle_id`,
       `g`.`permissions`,
       `hp_cc`.`unique_id`                 AS `hp_cc_unique_id`,
       `hp_cc`.`name`                      AS `hp_cc_name`,
       `hp_cc`.`display_name`              AS `hp_cc_display_name`,
       `hp_cc`.`sanitized_name`            AS `hp_cc_sanitized_name`,
       `hp_cc`.`source`                    AS `hp_cc_source`,
       `hp_cc`.`description`               AS `hp_cc_description`,
       `hp_cc`.`settings`                  AS `hp_cc_settings`,
       `hp_cc`.`config`                    AS `hp_cc_config`,
       `hp_cc`.`contact_addressbook`       AS `hp_cc_contact_addressbook`,
       `hp_cc`.`contact_groupname`         AS `hp_cc_contact_groupname`,
       `hp_cc`.`creation`                  AS `hp_cc_creation`,
       `hp_cc_wn`.`circle_id`              AS `hp_cc_wn_circle_id`,
       `hp_cc_wn`.`member_id`              AS `hp_cc_wn_member_id`,
       `hp_cc_wn`.`single_id`              AS `hp_cc_wn_single_id`,
       `hp_cc_wn`.`user_id`                AS `hp_cc_wn_user_id`,
       `hp_cc_wn`.`instance`               AS `hp_cc_wn_instance`,
       `hp_cc_wn`.`user_type`              AS `hp_cc_wn_user_type`,
       `hp_cc_wn`.`level`                  AS `hp_cc_wn_level`,
       `hp_cc_wn`.`status`                 AS `hp_cc_wn_status`,
       `hp_cc_wn`.`note`                   AS `hp_cc_wn_note`,
       `hp_cc_wn`.`contact_id`             AS `hp_cc_wn_contact_id`,
       `hp_cc_wn`.`cached_name`            AS `hp_cc_wn_cached_name`,
       `hp_cc_wn`.`cached_update`          AS `hp_cc_wn_cached_update`,
       `hp_cc_wn`.`contact_meta`           AS `hp_cc_wn_contact_meta`,
       `hp_cc_wn`.`joined`                 AS `hp_cc_wn_joined`,
       `hp_cc_wn_on`.`unique_id`           AS `hp_cc_wn_on_unique_id`,
       `hp_cc_wn_on`.`name`                AS `hp_cc_wn_on_name`,
       `hp_cc_wn_on`.`display_name`        AS `hp_cc_wn_on_display_name`,
       `hp_cc_wn_on`.`sanitized_name`      AS `hp_cc_wn_on_sanitized_name`,
       `hp_cc_wn_on`.`source`              AS `hp_cc_wn_on_source`,
       `hp_cc_wn_on`.`description`         AS `hp_cc_wn_on_description`,
       `hp_cc_wn_on`.`settings`            AS `hp_cc_wn_on_settings`,
       `hp_cc_wn_on`.`config`              AS `hp_cc_wn_on_config`,
       `hp_cc_wn_on`.`contact_addressbook` AS `hp_cc_wn_on_contact_addressbook`,
       `hp_cc_wn_on`.`contact_groupname`   AS `hp_cc_wn_on_contact_groupname`,
       `hp_cc_wn_on`.`creation`            AS `hp_cc_wn_on_creation`
FROM   `*prefix*group_folders_groups` `g`
       LEFT JOIN `*prefix*circles_circle` `hp_cc`
              ON `hp_cc`.`unique_id` = `g`.`circle_id`
       LEFT JOIN `*prefix*circles_member` `hp_cc_wn`
              ON ( `hp_cc_wn`.`circle_id` = `hp_cc`.`unique_id` )
                 AND ( `hp_cc_wn`.`level` = :dcValue1 )
       LEFT JOIN `*prefix*circles_circle` `hp_cc_wn_on`
              ON `hp_cc_wn_on`.`unique_id` = `hp_cc_wn`.`single_id`

where hp_cc_wn_on_contact_addressbook is 31 characters.

Checklist

… Oracle limitations

Signed-off-by: provokateurin <kate@provokateurin.de>
@provokateurin
Copy link
Member Author

/backport to stable30

@provokateurin
Copy link
Member Author

/backport to stable29

@provokateurin
Copy link
Member Author

/backport to stable28

Copy link
Contributor

@come-nc come-nc left a comment

Choose a reason for hiding this comment

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

This does not fix the bug but ensures it crashes on all DB, right?
Is there no way to magically fix the query instead, ideally in a class specific to OCI? I see we have Adapter, ExpressionBuilder and FuncitonBuilder specifc to OCI.

If we keep this as-is, what would the fix look like in the calling method?

@provokateurin
Copy link
Member Author

This does not fix the bug but ensures it crashes on all DB, right?

It doesn't crash, it just logs an error. It will fail on Oracle when the query is executed eventually.

Is there no way to magically fix the query instead, ideally in a class specific to OCI? I see we have Adapter, ExpressionBuilder and FuncitonBuilder specifc to OCI.

I don't think we can magically fix this as the code receiving the results relies on the exact names.

If we keep this as-is, what would the fix look like in the calling method?

The aliases just need to be shorter. For example Circles adds a ton of prefixes to the name so I think for that case we could maybe drop some of them.
In theory we could still have a columns with names longer than 30 chars, but those need to be prevented at creation time.

Copy link
Contributor

@come-nc come-nc left a comment

Choose a reason for hiding this comment

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

Okay I missed that the Exception is not thrown but only logged.

@nickvergessen
Copy link
Member

This check should also be extended to table names and schema names, but I'm not sure how to do that best.

We do this already:

/**
* Naming constraints:
* - Tables names must be 30 chars or shorter (27 + oc_ prefix)
* - Column names must be 30 chars or shorter
* - Index names must be 30 chars or shorter
* - Sequence names must be 30 chars or shorter
* - Primary key names must be set or the table name 23 chars or shorter
*
* Data constraints:
* - Tables need a primary key (Not specific to Oracle, but required for performant clustering support)
* - Columns with "NotNull" can not have empty string as default value
* - Columns with "NotNull" can not have number 0 as default value
* - Columns with type "bool" (which is in fact integer of length 1) can not be "NotNull" as it can not store 0/false
* - Columns with type "string" can not be longer than 4.000 characters, use "text" instead
*
* @see https://github.com/nextcloud/documentation/blob/master/developer_manual/basics/storage/database.rst
*
* @param Schema $sourceSchema
* @param Schema $targetSchema
* @param int $prefixLength
* @throws \Doctrine\DBAL\Exception
*/
public function ensureOracleConstraints(Schema $sourceSchema, Schema $targetSchema, int $prefixLength) {

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3. to review Waiting for reviews bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants