You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@ignite.apache.org by "zstan (via GitHub)" <gi...@apache.org> on 2023/06/30 11:22:43 UTC
[GitHub] [ignite-3] zstan opened a new pull request, #2275: IGNITE-19644 Change DROP|ADD COLUMN IF (NOT) EXISTS syntax
zstan opened a new pull request, #2275:
URL: https://github.com/apache/ignite-3/pull/2275
(no comment)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite-3] zstan commented on a diff in pull request #2275: IGNITE-19644 Change DROP|ADD COLUMN IF (NOT) EXISTS syntax
Posted by "zstan (via GitHub)" <gi...@apache.org>.
zstan commented on code in PR #2275:
URL: https://github.com/apache/ignite-3/pull/2275#discussion_r1250373230
##########
modules/runner/src/integrationTest/java/org/apache/ignite/internal/sql/internal/InternalSchemaTest.java:
##########
@@ -93,12 +92,23 @@ public void testDropColumns() {
checkDdl(true, ses, "ALTER TABLE my ADD COLUMN (c2 INT, c4 VARCHAR)");
+ ses.execute(
+ null,
+ "INSERT INTO my VALUES (2, '2', 2, '3')"
+ );
+
res = ses.execute(
null,
- "SELECT c1, c3 FROM my"
+ "SELECT c2, c4 FROM my WHERE c1=2"
);
- assertNotNull(res.next());
+ SqlRow result = res.next();
+
+ assertNotNull(result);
+ System.err.println(result.metadata().columns());
Review Comment:
yep, fixed
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite-3] lowka commented on a diff in pull request #2275: IGNITE-19644 Change DROP|ADD COLUMN IF (NOT) EXISTS syntax
Posted by "lowka (via GitHub)" <gi...@apache.org>.
lowka commented on code in PR #2275:
URL: https://github.com/apache/ignite-3/pull/2275#discussion_r1248100317
##########
modules/runner/src/integrationTest/java/org/apache/ignite/internal/sql/internal/InternalSchemaTest.java:
##########
@@ -93,12 +92,23 @@ public void testDropColumns() {
checkDdl(true, ses, "ALTER TABLE my ADD COLUMN (c2 INT, c4 VARCHAR)");
+ ses.execute(
+ null,
+ "INSERT INTO my VALUES (2, '2', 2, '3')"
+ );
+
res = ses.execute(
null,
- "SELECT c1, c3 FROM my"
+ "SELECT c2, c4 FROM my WHERE c1=2"
);
- assertNotNull(res.next());
+ SqlRow result = res.next();
+
+ assertNotNull(result);
+ System.err.println(result.metadata().columns());
Review Comment:
I think you forgot to remove this line.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite-3] zstan commented on pull request #2275: IGNITE-19644 Change DROP|ADD COLUMN IF (NOT) EXISTS syntax
Posted by "zstan (via GitHub)" <gi...@apache.org>.
zstan commented on PR #2275:
URL: https://github.com/apache/ignite-3/pull/2275#issuecomment-1618157111
@AMashenkov done
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite-3] AMashenkov commented on a diff in pull request #2275: IGNITE-19644 Change DROP|ADD COLUMN IF (NOT) EXISTS syntax
Posted by "AMashenkov (via GitHub)" <gi...@apache.org>.
AMashenkov commented on code in PR #2275:
URL: https://github.com/apache/ignite-3/pull/2275#discussion_r1251611060
##########
modules/runner/src/integrationTest/java/org/apache/ignite/internal/sql/engine/ItCreateTableDdlTest.java:
##########
@@ -122,6 +129,55 @@ public void implicitColocationColumns() {
assertEquals("ID0", colocationColumns[1].name());
}
+ /** Test correct mapping schema after alter columns. */
+ @Test
+ public void testDropAndAddColumns() {
+ sql("CREATE TABLE my (c1 INT PRIMARY KEY, c2 INT, c3 VARCHAR)");
+
+ sql("INSERT INTO my VALUES (1, 2, '3')");
+
+ List<List<Object>> res = sql("SELECT c1, c3 FROM my");
+
+ assertFalse(res.isEmpty());
+
+ sql("ALTER TABLE my DROP COLUMN c2");
+
+ res = sql("SELECT c1, c3 FROM my");
+
+ assertFalse(res.isEmpty());
+
+ sql("ALTER TABLE my ADD COLUMN (c2 INT, c4 VARCHAR)");
Review Comment:
Let's have a test for adding/dropping of individual column as well as adding/dropping of multiple columns.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite-3] zstan merged pull request #2275: IGNITE-19644 Change DROP|ADD COLUMN IF (NOT) EXISTS syntax
Posted by "zstan (via GitHub)" <gi...@apache.org>.
zstan merged PR #2275:
URL: https://github.com/apache/ignite-3/pull/2275
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite-3] lowka commented on a diff in pull request #2275: IGNITE-19644 Change DROP|ADD COLUMN IF (NOT) EXISTS syntax
Posted by "lowka (via GitHub)" <gi...@apache.org>.
lowka commented on code in PR #2275:
URL: https://github.com/apache/ignite-3/pull/2275#discussion_r1251894868
##########
modules/runner/src/integrationTest/java/org/apache/ignite/internal/sql/engine/ItCreateTableDdlTest.java:
##########
@@ -122,6 +129,63 @@ public void implicitColocationColumns() {
assertEquals("ID0", colocationColumns[1].name());
}
+ /** Test correct mapping schema after alter columns. */
+ @Test
+ public void testDropAndAddColumns() {
+ sql("CREATE TABLE my (c1 INT PRIMARY KEY, c2 INT, c3 VARCHAR)");
+
+ sql("INSERT INTO my VALUES (1, 2, '3')");
+
+ List<List<Object>> res = sql("SELECT c1, c3 FROM my");
+
+ assertFalse(res.isEmpty());
+
+ sql("ALTER TABLE my DROP COLUMN c2");
+
+ res = sql("SELECT c1, c3 FROM my");
+
+ assertFalse(res.isEmpty());
+
+ sql("ALTER TABLE my ADD COLUMN (c2 INT, c4 VARCHAR)");
+
+ sql("INSERT INTO my VALUES (2, '2', 2, '3')");
+
+ res = sql("SELECT c2, c4 FROM my WHERE c1=2");
+
+ assertEquals(2, res.get(0).get(0));
+
+ sql("ALTER TABLE my DROP COLUMN c4");
+ sql("ALTER TABLE my ADD COLUMN (c4 INT)");
+ sql("INSERT INTO my VALUES (3, '2', 3, 3)");
+
+ res = sql("SELECT c4 FROM my WHERE c1=3");
+
+ assertEquals(3, res.get(0).get(0));
+ }
+
+ /**
+ * Checks that schema version is updated even if column names are intersected.
+ */
+ // Need to be removed after https://issues.apache.org/jira/browse/IGNITE-19082
+ @Test
+ public void checkSchemaUpdatedWithEqAlterColumn() {
+ sql("CREATE TABLE TEST(ID INT PRIMARY KEY, VAL0 INT)");
+
+ Ignite node = CLUSTER_NODES.get(0);
+
+ ConfigurationManager cfgMgr = IgniteTestUtils.getFieldValue(node, "clusterCfgMgr");
+
+ TablesConfiguration tablesConfiguration = cfgMgr.configurationRegistry().getConfiguration(TablesConfiguration.KEY);
+
+ int schIdBefore = ((ExtendedTableView) tablesConfiguration.tables().get("TEST").value()).schemaId();
+
+ sql("ALTER TABLE TEST ADD COLUMN (VAL1 INT)");
+
Review Comment:
Maybe I am missing something. But nonetheless what could possibly go wrong in this test case (VAL0` and `VAL1` are distinct names).
Why this test case should be removed after some general issue related to catalog is resolved?
Maybe the link is not correct (https://issues.apache.org/jira/browse/IGNITE-19082 it's general issue for catalog migration).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite-3] zstan commented on a diff in pull request #2275: IGNITE-19644 Change DROP|ADD COLUMN IF (NOT) EXISTS syntax
Posted by "zstan (via GitHub)" <gi...@apache.org>.
zstan commented on code in PR #2275:
URL: https://github.com/apache/ignite-3/pull/2275#discussion_r1251995011
##########
modules/runner/src/integrationTest/java/org/apache/ignite/internal/sql/engine/ItCreateTableDdlTest.java:
##########
@@ -122,6 +129,63 @@ public void implicitColocationColumns() {
assertEquals("ID0", colocationColumns[1].name());
}
+ /** Test correct mapping schema after alter columns. */
+ @Test
+ public void testDropAndAddColumns() {
+ sql("CREATE TABLE my (c1 INT PRIMARY KEY, c2 INT, c3 VARCHAR)");
+
+ sql("INSERT INTO my VALUES (1, 2, '3')");
+
+ List<List<Object>> res = sql("SELECT c1, c3 FROM my");
+
+ assertFalse(res.isEmpty());
+
+ sql("ALTER TABLE my DROP COLUMN c2");
+
+ res = sql("SELECT c1, c3 FROM my");
+
+ assertFalse(res.isEmpty());
+
+ sql("ALTER TABLE my ADD COLUMN (c2 INT, c4 VARCHAR)");
+
+ sql("INSERT INTO my VALUES (2, '2', 2, '3')");
+
+ res = sql("SELECT c2, c4 FROM my WHERE c1=2");
+
+ assertEquals(2, res.get(0).get(0));
+
+ sql("ALTER TABLE my DROP COLUMN c4");
+ sql("ALTER TABLE my ADD COLUMN (c4 INT)");
+ sql("INSERT INTO my VALUES (3, '2', 3, 3)");
+
+ res = sql("SELECT c4 FROM my WHERE c1=3");
+
+ assertEquals(3, res.get(0).get(0));
+ }
+
+ /**
+ * Checks that schema version is updated even if column names are intersected.
+ */
+ // Need to be removed after https://issues.apache.org/jira/browse/IGNITE-19082
+ @Test
+ public void checkSchemaUpdatedWithEqAlterColumn() {
+ sql("CREATE TABLE TEST(ID INT PRIMARY KEY, VAL0 INT)");
+
+ Ignite node = CLUSTER_NODES.get(0);
+
+ ConfigurationManager cfgMgr = IgniteTestUtils.getFieldValue(node, "clusterCfgMgr");
+
+ TablesConfiguration tablesConfiguration = cfgMgr.configurationRegistry().getConfiguration(TablesConfiguration.KEY);
+
+ int schIdBefore = ((ExtendedTableView) tablesConfiguration.tables().get("TEST").value()).schemaId();
+
+ sql("ALTER TABLE TEST ADD COLUMN (VAL1 INT)");
+
Review Comment:
this test is uninformative as for me, but Andrey say not touch it, issue is correct, after migration there is no version logic as present in reflection.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite-3] zstan commented on pull request #2275: IGNITE-19644 Change DROP|ADD COLUMN IF (NOT) EXISTS syntax
Posted by "zstan (via GitHub)" <gi...@apache.org>.
zstan commented on PR #2275:
URL: https://github.com/apache/ignite-3/pull/2275#issuecomment-1614570347
@AMashenkov can u make a review plz ?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org