Skip to content

[3.x] Other schema #271

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

Open
wants to merge 5 commits into
base: 3.x-dev
Choose a base branch
from
Open

Conversation

softarius
Copy link

@softarius softarius commented Sep 20, 2022

Pull Request for Issue #322

Summary of Changes

Support other then default schema for columns getting

Testing Instructions

In database:
create schema foo;
create table foo.test (id integer, name varchar(10));

In Joomla:
Call getTableColumns method for this table 'foo.test'.

Documentation Changes Required

@richard67
Copy link
Contributor

richard67 commented Aug 15, 2024

As far as I can see by review, this PR will not work if either the schema name or the table name or both contain a dot, which is quite valid when the name which has a dot is name quoted. E.g. "my.schema"."my.table" is a valid schema, table combination.

Another question is why does this PR add that functionality only to the getTableColumns method and not to any other method which takes a table name as argument.

So in my opinion this PR should not be merged as it is. But other opinions are welcome.

@richard67 richard67 changed the base branch from 2.0-dev to 3.x-dev March 23, 2025 12:57
@richard67 richard67 changed the title Other schema [3.x] Other schema Mar 23, 2025
@richard67
Copy link
Contributor

@softarius It seems you had rebased your branch locally to 3.x-dev and pushed the changes, but the PR on GitHub was still against the 2.0-dev branch. That's why it has shown so many unrelated changes.

I have fixed that now by rebasing to 3.x-dev on GitHub. Now it look ok again.

As far as I can see you have also resolved my previous comment. I will check and review again when I have more time.

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

Successfully merging this pull request may close these issues.

2 participants