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

changeColumnSQL not handling new vs changed columns correctly #1010

Closed
1 task done
ekariz opened this issue Nov 10, 2023 · 14 comments
Closed
1 task done

changeColumnSQL not handling new vs changed columns correctly #1010

ekariz opened this issue Nov 10, 2023 · 14 comments
Labels
bug data dictionary datadict feature
Milestone

Comments

@ekariz
Copy link

ekariz commented Nov 10, 2023

Description

AddColumnSQL function missing for mysql. I cannot add a new column / alter mysql table

Environment

  • ADOdb version: Version number (or Git Commit/Ref)
  • Driver or Module: mysql
  • Database type and version: mysql Ver 8
  • PHP version: 8.2
  • Platform: Linux (Ubuntu 20)
  • I have tested that the problem is reproducible in the latest release / master branch / hotfix branch
    ⚠️ Keep only the applicable option(s), i.e. where you actually tested and remove the others)

Steps to reproduce

Detailed, step-by-step instructions to reproduce the behavior, including:

$schema = new adoSchema($this->dbconn);

// Post Execution (default)
$schema->executeInline(false);
$schema->parseSchemaFile('schema.xml');
$schema->executeSchema();

if you add a new field in schema.xml , if you execute the datadictionary again , the new column is not added in the db table.

Expected behavior

Adding new column in xml to mysql table

Additional context

This was working with older ADOdb versions

@ekariz ekariz added the triage New issues not yet reviewed by ADOdb developers label Nov 10, 2023
@dregad
Copy link
Member

dregad commented Nov 10, 2023

Can you please

  • provide a sample XML file (before/after) so we have a consistent reproducible test case,
  • describe how this->dbconn is initialized
  • indicate which ADOdb version you're using, and in which "older ADOdb versions" this was previously working

@ekariz
Copy link
Author

ekariz commented Nov 14, 2023

XML before running in db

image

Resulting SQL

image

Resulting mysql table
image

Added more columns in xml

image

Resulting SQL AFTER adding column in XML

image

MYSQL Table not modified to ADD new columns

image

In Summary, i expect this
ALTER TABLE tbl_test ADD COLUMN column2 VARCHAR(30) NOT NULL

not this
ALTER TABLE tbl_test MODIFY COLUMN column2 VARCHAR(30) NOT NULL

@ekariz
Copy link
Author

ekariz commented Nov 14, 2023

This is my adodb version
v5.22.5 2023-04-03

connection method
image

@ekariz
Copy link
Author

ekariz commented Nov 14, 2023

adodb_logsql errors
image

@dregad
Copy link
Member

dregad commented Nov 14, 2023

Thanks for the details. I'll have a look as time allows.

Note: For next time, better paste text (XML, SQL, PHP...) instead of images so I can copy-paste instead of re-type.

This is my adodb version
v5.22.5 2023-04-03

I suppose this is the version that has the problem ? You did not indicate which was the older ADOdb version that was working fine.

@ekariz
Copy link
Author

ekariz commented Nov 14, 2023

First XML

<table name="tbl_test2">
  <descr>test table</descr>
  <field name="id"            type="I" size="10"   ><NOTNULL/></field>
  <field name="column1"       type="C" size="30"   ><NOTNULL/></field>

</table>

SQL

 CREATE TABLE tbl_test2 (
id    INTEGER(10) NOT NULL,
column1    VARCHAR(30) NOT NULL
)

Updated XML

<table name="tbl_test2">
  <descr>test table</descr>
  <field name="id"            type="I" size="10"   ><NOTNULL/></field>
  <field name="column1"       type="C" size="30"   ><NOTNULL/></field>
  <field name="column2"       type="C" size="30"   ><NOTNULL/></field>
  <field name="column3"       type="C" size="30"   ><NOTNULL/></field>

</table>

SQL

ALTER TABLE tbl_test2 MODIFY COLUMN id INTEGER(10) NOT NULL;
ALTER TABLE tbl_test2 MODIFY COLUMN column1 VARCHAR(30) NOT NULL;
ALTER TABLE tbl_test2 MODIFY COLUMN column2 VARCHAR(30) NOT NULL;
ALTER TABLE tbl_test2 MODIFY COLUMN column3 VARCHAR(30) NOT NULL;

@mnewnham mnewnham added data dictionary datadict feature bug and removed xml schema labels Nov 16, 2023
@mnewnham
Copy link
Contributor

It looks like a problem with the way that ADODB_DataDict::changeTableSQL() is structured, when it iterates over the columns of an existing table, it doesn't distinguish between columns that already exist and those that don't.

This looks like a problem caused by This commit

@mnewnham mnewnham removed the triage New issues not yet reviewed by ADOdb developers label Nov 16, 2023
@dregad
Copy link
Member

dregad commented Nov 17, 2023

Thanks for the analysis @mnewnham, that was on my to-do list for this week-end. Are you planning to work on a fix ?

@mnewnham
Copy link
Contributor

@dregad I won't be able to get to it in the next 7 days

@ekariz
Copy link
Author

ekariz commented May 7, 2024

Hi, just a follow up on this issue. Is a there a possible fix being worked on ?

@dregad
Copy link
Member

dregad commented May 8, 2024

Hi @ekariz this fell off the radar, I have not looked at it since November last year. I'm definitely not working on it at all, and won't have time to do so in the short term, sorry.

@mnewnham
Copy link
Contributor

I've looked at this again and you seem to have fixed this with 8d88183 @dregad

@dregad
Copy link
Member

dregad commented May 26, 2024

I've looked at this again and you seem to have fixed this with 8d88183 @dregad

Indeed 😄

That should have been committed in hotfix/5.22 branch though... I'll back-port it.

While retesting, I noticed that I forgot to initialize the $fields_to_add and $fields_to_alter arrays, resultsing in PHP warnings:

Notice: Undefined variable: fields_to_add in ./adodb-datadict.inc.php on line 1087
PHP Warning:  Invalid argument supplied for foreach() in ./adodb-datadict.inc.php on line 679

Also, I believe there is something wrong with the logic to drop columns when $dropOldFlds is true at the end of the changeTableSQL() method, as the loop refers to an undefined $lines array. Change was introduced by b5732c5. I'll open a separate issue to track this.

dregad added a commit that referenced this issue May 26, 2024
@dregad dregad added this to the 5.22.8 milestone May 26, 2024
@dregad dregad closed this as completed May 26, 2024
@dregad dregad changed the title AddColumnSQL missing in Data Dictionary for MySQL changeColumnSQL not handling new vs changed columns correctly May 26, 2024
@ekariz
Copy link
Author

ekariz commented May 26, 2024

Thanks @dregad and @mnewnham

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug data dictionary datadict feature
Projects
None yet
Development

No branches or pull requests

3 participants