Skip to content

Add DBAL Schema wrapper in OCP - #63013

Open
CarlSchwan wants to merge 5 commits into
masterfrom
carl/dbal-wrapper
Open

Add DBAL Schema wrapper in OCP#63013
CarlSchwan wants to merge 5 commits into
masterfrom
carl/dbal-wrapper

Conversation

@CarlSchwan

@CarlSchwan CarlSchwan commented Aug 7, 2026

Copy link
Copy Markdown
Member

Summary

Same as the DBAL one aside that Column::getType returns a string instead of a DBAL Type. Column::setType support both a DBAL Type and a string.

TODO

  • ...

Checklist

AI (if applicable)

  • The content of this PR was partly or fully generated using AI

@CarlSchwan CarlSchwan added this to the Nextcloud 35 milestone Aug 7, 2026
@CarlSchwan CarlSchwan added the 3. to review Waiting for reviews label Aug 7, 2026
@CarlSchwan CarlSchwan self-assigned this Aug 7, 2026
@CarlSchwan
CarlSchwan marked this pull request as ready for review August 7, 2026 10:14
@CarlSchwan
CarlSchwan requested review from leftybournes, nfebe, provokateurin and salmart-dev and removed request for a team August 7, 2026 10:14

@come-nc come-nc left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Changing return type of getTable is breaking API, no?

Comment thread apps/federatedfilesharing/lib/Migration/Version1011Date20201120125158.php Outdated
@CarlSchwan

Copy link
Copy Markdown
Member Author

Changing return type of getTable is breaking API, no?

It's a consumable API so it should be fine as long as the returned object behaves the same as before

@ChristophWurst ChristophWurst left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Public API looks sane 👍

We could improve the strictness but that will break existing code

Comment thread lib/public/DB/Schema/IColumn.php Outdated
Comment thread lib/public/DB/Schema/IColumn.php Outdated
Comment thread lib/public/DB/Schema/IIndex.php Outdated
Comment thread lib/public/DB/Schema/IIndex.php Outdated
* @throws SchemaException
* @since 35.0.0
*/
public function addIndex(array $columnNames, ?string $indexName = null, array $flags = [], array $options = []): self;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

BC break but in terms of typing it would be nicer if this method takes an IIndex with its typed setters instead

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I'm trying my best to not have BC breaks. Maybe we can add another method called createIndex which take a IIndex. and deprecate this addIndex method.

Same as the DBAL one aside that Column::getType returns a string instead
of a DBAL Type. Column::setType support both a DBAL Type and a string.

Signed-off-by: Carl Schwan <carl@carlschwan.eu>
Signed-off-by: Carl Schwan <carl@carlschwan.eu>
Signed-off-by: Carl Schwan <carl@carlschwan.eu>
Prefer the fully typed methods instead and there is always the __call
magic method as fallback for apps.

Signed-off-by: Carl Schwan <carl@carlschwan.eu>
Signed-off-by: Carl Schwan <carl@carlschwan.eu>
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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants