feat: implement ALTER TYPE oldName NAME newName operation by daffodilistic · Pull Request #1501 · ArcadeData/arcadedb

daffodilistic · 2024-03-06T04:25:42Z

What does this PR do?

Implement ALTER TYPE oldName NAME newName

Motivation

It's listed in the documentation page, but its not working in the database

Related issues

N/A

Additional Notes

N/A

Checklist

I have run the build using mvn clean package command
My unit tests cover both failure and success scenarios

lvca

Thanks for this PR! You're right, we never completed the rename of the type. Have you already tested it? We'd need some test cases too. Let me know if you can do it, or we can find somebody can help with that.

engine/src/main/java/com/arcadedb/query/sql/parser/AlterTypeStatement.java

daffodilistic · 2024-03-07T22:51:34Z

Thanks for this PR! You're right, we never completed the rename of the type. Have you already tested it? We'd need some test cases too. Let me know if you can do it, or we can find somebody can help with that.

I'm not sure where the tests are located (e2e folder?). Some guidance would be good :)
Also documentation update on the underlying mechanics has to be done too.

lvca · 2024-03-09T00:20:16Z

Also, in the case of indexes, we have to rename them as well. Not fundamental, because they are linked in the schema.json so it would still work with the old names.

The only important thing missing is, at least, one test case to avoid regression and to test that this change works.

lvca · 2024-04-26T18:26:52Z

engine/src/test/java/com/arcadedb/query/sql/executor/AlterTypeExecutionTest.java

+  public void sqlAlterTypeName() {
+    database.command("sql", "CREATE VERTEX TYPE Mpv");
+    database.command("sql", "ALTER TYPE Mpv NAME Sedan");
+    Assertions.assertNotNull(database.getSchema().getType("Sedan"));


It would be wonderful if you can also test the previous Mpv type doesn't exist anymore

lvca · 2024-04-26T18:27:31Z

engine/src/test/java/com/arcadedb/query/sql/executor/AlterTypeExecutionTest.java

+
+  @Test
+  public void sqlAlterTypeName() {
+    database.command("sql", "CREATE VERTEX TYPE Mpv");


Could you please create a property with an index, so see if everything works as expected? Thanks in advance @daffodilistic !

…ypes

lvca · 2024-05-14T13:09:00Z

engine/src/test/java/com/arcadedb/query/sql/executor/AlterTypeExecutionTest.java

+    database.command("sql", "CREATE INDEX ON Sedan(engine_number) UNIQUE");
+    database.begin();
+    // insert a random record
+    database.getSchema().getType("Sedan").newRecord().set("engine_number", "123").save();


Thanks for the change. I meant creating an index on the old type and see what happens when you rename it. Is the index still working or does it need to be renamed too?

daffodilistic · 2024-10-12T04:43:01Z

Hi,

I'm getting an error with the latest changes to the test case. It seems like there's an issue with existing documents of the type with a custom index, before being renamed:

[ERROR] com.arcadedb.query.sql.executor.AlterTypeExecutionTest.sqlAlterTypeName -- Time elapsed: 0.312 s <<< ERROR!
java.lang.ArithmeticException: / by zero
        at com.arcadedb.schema.LocalSchema.copyType(LocalSchema.java:379)
        at com.arcadedb.query.sql.parser.AlterTypeStatement.executeDDL(AlterTypeStatement.java:120)
        at com.arcadedb.query.sql.executor.DDLExecutionPlan.executeInternal(DDLExecutionPlan.java:60)
        at com.arcadedb.query.sql.parser.DDLStatement.execute(DDLStatement.java:49)
        at com.arcadedb.query.sql.parser.Statement.execute(Statement.java:65)
        at com.arcadedb.query.sql.SQLQueryEngine.command(SQLQueryEngine.java:113)
        at com.arcadedb.database.LocalDatabase.command(LocalDatabase.java:1327)
        at com.arcadedb.query.sql.executor.AlterTypeExecutionTest.sqlAlterTypeName(AlterTypeExecutionTest.java:129)
        at java.base/java.lang.reflect.Method.invoke(Method.java:566)
        at java.base/java.util.ArrayList.forEach(ArrayList.java:1541)
        at java.base/java.util.ArrayList.forEach(ArrayList.java:1541)

Not quite sure what to make of it. I'll try syncing my branch with upstream master first to see if any bugfixes resolved it

#1501 (comment)

lvca · 2024-10-21T02:17:25Z

That issue should be fixed now in main.

daffodilistic · 2024-10-22T09:52:18Z

Should this PR be closed and merged, or a new one created? The test passed with the latest version on main branch

lvca · 2024-10-22T13:49:15Z

That works, thanks!

feat: implement ALTER TYPE oldName NAME newName operation

dd4a17f

lvca self-requested a review March 6, 2024 15:06

lvca added the enhancement New feature or request label Mar 6, 2024

lvca reviewed Mar 6, 2024

View reviewed changes

engine/src/main/java/com/arcadedb/query/sql/parser/AlterTypeStatement.java Outdated Show resolved Hide resolved

engine/src/main/java/com/arcadedb/query/sql/parser/AlterTypeStatement.java Show resolved Hide resolved

daffodilistic added 2 commits March 8, 2024 06:53

fix: whoops, forgot to break out of switch-case

fc726ee

fix: remove extraneous check for type existence before copy operation

a44befb

lvca assigned daffodilistic Mar 9, 2024

lvca self-requested a review March 9, 2024 00:20

test: add test for ALTER TYPE oldName NAME newName

e43240d

lvca requested changes Apr 26, 2024

View reviewed changes

daffodilistic added 2 commits May 13, 2024 15:52

test: add test to confirm previous type doesn't exist when renaming t…

9acecea

…ypes

test: add test for indexing on renamed types

79d9373

lvca reviewed May 14, 2024

View reviewed changes

test: add property and index on old type, before renaming

c5fdf71

Merge branch 'ArcadeData:main' into dev-daffodilistic

2e5a6c2

lvca added a commit that referenced this pull request Oct 21, 2024

fix: fixed ArithmeticException: / by zero

c9b5bda

#1501 (comment)

lvca added this to the 24.11.1 milestone Oct 22, 2024

lvca closed this Oct 22, 2024

lvca reopened this Oct 22, 2024

lvca merged commit 22e08d2 into ArcadeData:main Oct 22, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

feat: implement ALTER TYPE oldName NAME newName operation#1501

feat: implement ALTER TYPE oldName NAME newName operation#1501
lvca merged 8 commits intoArcadeData:mainfrom
daffodilistic:dev-daffodilistic

daffodilistic commented Mar 6, 2024

Uh oh!

lvca left a comment

Uh oh!

Uh oh!

Uh oh!

daffodilistic commented Mar 7, 2024

Uh oh!

lvca commented Mar 9, 2024

Uh oh!

lvca Apr 26, 2024

Uh oh!

lvca Apr 26, 2024

Uh oh!

lvca May 14, 2024

Uh oh!

daffodilistic May 16, 2024

Uh oh!

daffodilistic commented Oct 12, 2024

Uh oh!

lvca commented Oct 21, 2024

Uh oh!

daffodilistic commented Oct 22, 2024

Uh oh!

lvca commented Oct 22, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

daffodilistic commented Mar 6, 2024

What does this PR do?

Motivation

Related issues

Additional Notes

Checklist

Uh oh!

lvca left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

daffodilistic commented Mar 7, 2024

Uh oh!

lvca commented Mar 9, 2024

Uh oh!

lvca Apr 26, 2024

Choose a reason for hiding this comment

Uh oh!

lvca Apr 26, 2024

Choose a reason for hiding this comment

Uh oh!

lvca May 14, 2024

Choose a reason for hiding this comment

Uh oh!

daffodilistic May 16, 2024

Choose a reason for hiding this comment

Uh oh!

daffodilistic commented Oct 12, 2024

Uh oh!

lvca commented Oct 21, 2024

Uh oh!

daffodilistic commented Oct 22, 2024

Uh oh!

lvca commented Oct 22, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants