Skip to content

Commit 6e63d43

Browse files
committed
fix: [sql] issues with MOVE VERTEX command
Fixed issue ArcadeData#4461 The native ANTLR SQL parser's MOVE VERTEX support was incomplete on multiple fronts. I reproduced every reported case, fixed the root causes, and added regression tests.
1 parent d342697 commit 6e63d43

4 files changed

Lines changed: 135 additions & 31 deletions

File tree

engine/src/main/antlr4/com/arcadedb/query/sql/grammar/SQLParser.g4

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -421,13 +421,16 @@ deleteFunctionStatement
421421

422422
/**
423423
* MOVE VERTEX statement
424-
* MOVE VERTEX source TO target [SET ...] [MERGE ...]
425-
* Supports: MOVE VERTEX expr TO type:TypeName or TO TypeName or TO BUCKET:name
424+
* MOVE VERTEX source TO target [SET ...|REMOVE ...|MERGE ...|CONTENT ...] [BATCH n]
425+
* Supports: MOVE VERTEX expr TO type:TypeName or TO TypeName or TO bucket:name or TO bucket:id
426426
*/
427427
moveVertexStatement
428-
: MOVE VERTEX expression TO (TYPE COLON identifier | BUCKET COLON identifier | identifier)
429-
(SET updateItem (COMMA updateItem)*)?
430-
(MERGE expression)?
428+
: MOVE VERTEX expression TO ( TYPE COLON identifier
429+
| BUCKET COLON identifier
430+
| BUCKET_IDENTIFIER
431+
| BUCKET_NUMBER_IDENTIFIER
432+
| identifier )
433+
updateOperation?
431434
(BATCH expression)?
432435
;
433436

engine/src/main/java/com/arcadedb/query/sql/antlr/SQLASTBuilder.java

Lines changed: 18 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -4730,11 +4730,10 @@ else if (baseExpr.inputParam != null)
47304730
return stmt;
47314731
}
47324732

4733-
// TODO: Complete MOVE VERTEX visitor implementation - temporarily disabled
4734-
// /**
4735-
// * Visit MOVE VERTEX statement.
4736-
// * Supports: MOVE VERTEX expr TO type:TypeName or TO TypeName or TO bucket:name
4737-
// */
4733+
/**
4734+
* Visit MOVE VERTEX statement.
4735+
* Supports: MOVE VERTEX expr TO type:TypeName | TO bucket:name | TO bucket:id [SET|REMOVE|MERGE|CONTENT ...] [BATCH n]
4736+
*/
47384737
@Override
47394738
public MoveVertexStatement visitMoveVertexStmt(final SQLParser.MoveVertexStmtContext ctx) {
47404739
final MoveVertexStatement stmt = new MoveVertexStatement(-1);
@@ -4769,30 +4768,31 @@ public MoveVertexStatement visitMoveVertexStmt(final SQLParser.MoveVertexStmtCon
47694768
}
47704769
stmt.source = fromItem;
47714770

4772-
// Target: TYPE:identifier or BUCKET:identifier or identifier (bucket name)
4771+
// Target: TYPE:identifier, BUCKET:name, bucket:name (single token), bucket:id (single token) or bare identifier (bucket name)
47734772
if (moveCtx.TYPE() != null) {
47744773
// TO TYPE:typename
47754774
stmt.targetType = (Identifier) visit(moveCtx.identifier());
4775+
} else if (moveCtx.BUCKET_IDENTIFIER() != null) {
4776+
// TO bucket:name (lexed as a single BUCKET_IDENTIFIER token)
4777+
final String text = moveCtx.BUCKET_IDENTIFIER().getText();
4778+
stmt.targetBucket = new Bucket(text.substring("bucket:".length()));
4779+
} else if (moveCtx.BUCKET_NUMBER_IDENTIFIER() != null) {
4780+
// TO bucket:123 (lexed as a single BUCKET_NUMBER_IDENTIFIER token)
4781+
final String text = moveCtx.BUCKET_NUMBER_IDENTIFIER().getText();
4782+
stmt.targetBucket = new Bucket(Integer.parseInt(text.substring("bucket:".length())));
47764783
} else if (moveCtx.BUCKET() != null) {
4777-
// TO BUCKET:bucketname
4784+
// TO BUCKET : bucketname (separated tokens, e.g. with spaces around the colon)
47784785
final Identifier bucketId = (Identifier) visit(moveCtx.identifier());
47794786
stmt.targetBucket = new Bucket(bucketId.getStringValue());
47804787
} else {
4781-
// TO bucketname (without BUCKET: prefix)
4788+
// TO bucketname (without bucket: prefix)
47824789
final Identifier bucketId = (Identifier) visit(moveCtx.identifier());
47834790
stmt.targetBucket = new Bucket(bucketId.getStringValue());
47844791
}
47854792

4786-
// SET clause (updateItems)
4787-
if (moveCtx.SET() != null) {
4788-
final UpdateOperations updateOps = new UpdateOperations(-1);
4789-
updateOps.type = UpdateOperations.TYPE_SET;
4790-
for (final SQLParser.UpdateItemContext itemCtx : moveCtx.updateItem()) {
4791-
final UpdateItem item = (UpdateItem) visit(itemCtx);
4792-
updateOps.updateItems.add(item);
4793-
}
4794-
stmt.updateOperations = updateOps;
4795-
}
4793+
// Update operations: SET, ADD, PUT, REMOVE, INCREMENT, MERGE, CONTENT
4794+
if (moveCtx.updateOperation() != null)
4795+
stmt.updateOperations = (UpdateOperations) visit(moveCtx.updateOperation());
47964796

47974797
// BATCH clause
47984798
if (moveCtx.BATCH() != null) {

engine/src/main/java/com/arcadedb/query/sql/executor/MoveVertexStep.java

Lines changed: 14 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
*/
1919
package com.arcadedb.query.sql.executor;
2020

21+
import com.arcadedb.database.RID;
2122
import com.arcadedb.exception.TimeoutException;
2223
import com.arcadedb.graph.Vertex;
2324
import com.arcadedb.query.sql.parser.Bucket;
@@ -44,17 +45,22 @@ public MoveVertexStep(final Identifier targetType, final Bucket targetBucket, fi
4445
@Override
4546
public ResultSet syncPull(final CommandContext ctx, final int records) throws TimeoutException {
4647
final ResultSet prevResult = getPrev().syncPull(ctx, records);
48+
49+
// The moved vertices are re-created under the target type/bucket with a brand-new RID. Replace each input result with an
50+
// updatable result wrapping the new record so that any following SET/REMOVE/MERGE/CONTENT operations and the final save act
51+
// on the moved vertex, and so the new RID is returned to the caller.
52+
final InternalResultSet result = new InternalResultSet();
4753
while (prevResult.hasNext()) {
48-
final Result result = prevResult.next();
49-
final Vertex v = result.getVertex().get();
54+
final Result item = prevResult.next();
55+
final Vertex v = item.getVertex().orElse(null);
5056
if (v != null) {
51-
if (targetBucket != null)
52-
v.moveToBucket(targetBucket);
53-
else
54-
v.moveToType(targetType);
55-
}
57+
final RID newIdentity = targetBucket != null ? v.moveToBucket(targetBucket) : v.moveToType(targetType);
58+
final Vertex moved = ctx.getDatabase().lookupByRID(newIdentity, true).asVertex();
59+
result.add(new UpdatableResult(moved.modify()));
60+
} else
61+
result.add(item);
5662
}
57-
return prevResult;
63+
return result;
5864
}
5965

6066
@Override

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

Lines changed: 95 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -232,4 +232,99 @@ void moveVertexByRid() {
232232
assertThat(rs.hasNext()).isFalse();
233233
rs.close();
234234
}
235+
236+
/**
237+
* Regression test for issue #4461: MOVE VERTEX must return the new RID of the moved (re-created) vertex,
238+
* for both a literal RID source and a SELECT subquery source.
239+
*/
240+
@Test
241+
void moveVertexReturnsNewRid() {
242+
final String typeA = "testMoveRetA";
243+
final String typeB = "testMoveRetB";
244+
database.getSchema().createVertexType(typeA);
245+
database.getSchema().createVertexType(typeB);
246+
database.setAutoTransaction(true);
247+
248+
final String rid1 = database.command("sql", "create vertex " + typeA).next().getIdentity().get().toString();
249+
try (final ResultSet rs = database.command("sql", "MOVE VERTEX " + rid1 + " TO TYPE:" + typeB)) {
250+
assertThat(rs.hasNext()).isTrue();
251+
final Result moved = rs.next();
252+
assertThat(moved.getIdentity()).isPresent();
253+
assertThat(moved.getElement().get().getTypeName()).isEqualTo(typeB);
254+
assertThat(rs.hasNext()).isFalse();
255+
}
256+
257+
final String rid2 = database.command("sql", "create vertex " + typeA).next().getIdentity().get().toString();
258+
try (final ResultSet rs = database.command("sql", "MOVE VERTEX (SELECT FROM " + rid2 + ") TO TYPE:" + typeB)) {
259+
assertThat(rs.hasNext()).isTrue();
260+
final Result moved = rs.next();
261+
assertThat(moved.getIdentity()).isPresent();
262+
// the moved record is a brand-new record, not the source RID
263+
assertThat(moved.getIdentity().get().toString()).isNotEqualTo(rid2);
264+
assertThat(moved.getElement().get().getTypeName()).isEqualTo(typeB);
265+
assertThat(rs.hasNext()).isFalse();
266+
}
267+
}
268+
269+
/**
270+
* Regression test for issue #4461: MOVE VERTEX ... TO bucket:name must parse and move the vertex to the target bucket.
271+
*/
272+
@Test
273+
void moveVertexToBucket() {
274+
final String typeA = "testMoveBucketA";
275+
database.getSchema().createVertexType(typeA).addBucket(database.getSchema().createBucket("testMoveBucket_extra"));
276+
database.setAutoTransaction(true);
277+
278+
final String rid = database.command("sql", "create vertex " + typeA).next().getIdentity().get().toString();
279+
try (final ResultSet rs = database.command("sql", "MOVE VERTEX " + rid + " TO BUCKET:testMoveBucket_extra")) {
280+
assertThat(rs.hasNext()).isTrue();
281+
final Result moved = rs.next();
282+
assertThat(moved.getElement().get().getIdentity().getBucketId())
283+
.isEqualTo(database.getSchema().getBucketByName("testMoveBucket_extra").getFileId());
284+
assertThat(rs.hasNext()).isFalse();
285+
}
286+
}
287+
288+
/**
289+
* Regression test for issue #4461: the SET/REMOVE/MERGE/CONTENT operations must be applied to the moved (re-created)
290+
* vertex. Previously they were silently applied to the stale source record and lost.
291+
*/
292+
@Test
293+
void moveVertexWithUpdateOperations() {
294+
final String typeA = "testMoveOpsA";
295+
final String typeB = "testMoveOpsB";
296+
database.getSchema().createVertexType(typeA);
297+
database.getSchema().createVertexType(typeB);
298+
database.setAutoTransaction(true);
299+
300+
// SET
301+
String rid = database.command("sql", "create vertex " + typeA + " set somefield = 0").next().getIdentity().get().toString();
302+
try (final ResultSet rs = database.command("sql",
303+
"MOVE VERTEX (SELECT FROM " + rid + ") TO TYPE:" + typeB + " SET somefield = 1")) {
304+
assertThat(rs.next().<Integer>getProperty("somefield")).isEqualTo(1);
305+
}
306+
307+
// REMOVE
308+
rid = database.command("sql", "create vertex " + typeA + " set somefield = 0").next().getIdentity().get().toString();
309+
try (final ResultSet rs = database.command("sql",
310+
"MOVE VERTEX (SELECT FROM " + rid + ") TO TYPE:" + typeB + " REMOVE somefield")) {
311+
assertThat(rs.next().hasProperty("somefield")).isFalse();
312+
}
313+
314+
// MERGE
315+
rid = database.command("sql", "create vertex " + typeA + " set somefield = 0").next().getIdentity().get().toString();
316+
try (final ResultSet rs = database.command("sql",
317+
"MOVE VERTEX (SELECT FROM " + rid + ") TO TYPE:" + typeB + " MERGE {\"somefield\": 9}")) {
318+
assertThat(rs.next().<Integer>getProperty("somefield")).isEqualTo(9);
319+
}
320+
321+
// CONTENT
322+
rid = database.command("sql", "create vertex " + typeA + " set somefield = 0").next().getIdentity().get().toString();
323+
try (final ResultSet rs = database.command("sql",
324+
"MOVE VERTEX (SELECT FROM " + rid + ") TO TYPE:" + typeB + " CONTENT {\"newfield\": 7}")) {
325+
final Result moved = rs.next();
326+
assertThat(moved.<Integer>getProperty("newfield")).isEqualTo(7);
327+
assertThat(moved.hasProperty("somefield")).isFalse();
328+
}
329+
}
235330
}

0 commit comments

Comments
 (0)