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

test: adding tests for DML upsert features. #2778

Closed
wants to merge 16 commits into from

Conversation

arpan14
Copy link
Contributor

@arpan14 arpan14 commented Jan 10, 2024

  • Tests are currently failing since feature is not in production. Merge once it releases.

@arpan14 arpan14 requested a review from a team as a code owner January 10, 2024 07:32
@product-auto-label product-auto-label bot added size: m Pull request size is medium. api: spanner Issues related to the googleapis/java-spanner API. labels Jan 10, 2024
@arpan14 arpan14 requested a review from harshachinta January 10, 2024 07:54
.readRow("T", Key.of(String.format("%d-boo1", id)), Collections.singletonList("V")))
.isNull();
}

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can add 2 more tests as below. They cover cases like:

  • Insert or ignore ignores existing rows.
  • Insert or update inserts (not updates) in case of new rows.
@test
public void testInsertOrUpdateOnNewRow() {
if (dialect.dialect == Dialect.GOOGLE_STANDARD_SQL) {
executeUpdate(DML_COUNT_WITH_UPSERT, insertOrUpdateDml());
} else {
executeUpdate(DML_COUNT_WITH_UPSERT, insertOrUpdateDmlPostgresql());
}
assertThat(
getClient(dialect.dialect)
.singleUse(TimestampBound.strong())
.readRow("T", Key.of(String.format("%d-boo1", id)), Collections.singletonList("V"))
.getLong(0))
.isEqualTo(10);
assertThat(
getClient(dialect.dialect)
.singleUse(TimestampBound.strong())
.readRow("T", Key.of(String.format("%d-boo5", id)), Collections.singletonList("V"))
.getLong(0))
.isEqualTo(50);
// Delete the rows
executeUpdate(DML_COUNT_WITH_UPSERT, deleteDml());
assertThat(
getClient(dialect.dialect)
.singleUse(TimestampBound.strong())
.readRow("T", Key.of(String.format("%d-boo1", id)), Collections.singletonList("V")))
.isNull();
}

@test
public void testInsertOrIgnoreDmlOnExistingRow() {
// First insert few rows.
executeUpdate(DML_COUNT, insertDml());
assertThat(
getClient(dialect.dialect)
.singleUse(TimestampBound.strong())
.readRow("T", Key.of(String.format("%d-boo1", id)), Collections.singletonList("V"))
.getLong(0))
.isEqualTo(1);

// Existing rows are ignored, only 1 new row is inserted.
if (dialect.dialect == Dialect.GOOGLE_STANDARD_SQL) {
  executeUpdate(1, insertOrIgnoreDml());
} else {
  executeUpdate(1, insertOrIgnoreDmlPostgresql());
}
assertThat(
        getClient(dialect.dialect)
            .singleUse(TimestampBound.strong())
            .readRow("T", Key.of(String.format("%d-boo1", id)), Collections.singletonList("V"))
            .getLong(0))
    .isEqualTo(1);
assertThat(
        getClient(dialect.dialect)
            .singleUse(TimestampBound.strong())
            .readRow("T", Key.of(String.format("%d-boo5", id)), Collections.singletonList("V"))
            .getLong(0))
    .isEqualTo(5);
// Delete the rows
executeUpdate(DML_COUNT_WITH_UPSERT, deleteDml());
assertThat(
        getClient(dialect.dialect)
            .singleUse(TimestampBound.strong())
            .readRow("T", Key.of(String.format("%d-boo1", id)), Collections.singletonList("V")))
    .isNull();
}

"INSERT INTO T (k, v) VALUES ('%d-boo1', 10), ('%d-boo2', 20), ('%d-boo3', 30), "
+ "('%d-boo4', 40), ('%d-boo5', 50) "
+ "ON CONFLICT(k) DO UPDATE SET k = excluded.k, v = excluded.v;";
private static final String INSERT_IGNORE_DML_FROM_QUERY =

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you pls add ORDER BY v to each subquery as below:
SELECT .. FROM T ORDER BY v

There should be no change change to the result

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is the error due to ORDER BY ?

.readRow("T", Key.of(String.format("%d-boo1", id)), Collections.singletonList("V")))
.isNull();
}

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we 2 more cases for duplicate rows:

@Test
  public void testInsertOrIgnoreDuplicateRows() {
    long expectedRowCount = 2;

    // Only unique keys are inserted. Duplicate keys are ignored.
    if (dialect.dialect == Dialect.GOOGLE_STANDARD_SQL) {
      executeUpdate(
          expectedRowCount,
          "INSERT OR IGNORE INTO T (k, v) VALUES ('1-boo1', 1), ('2-boo2', 2), ('1-boo1', 1)");
    } else {
      executeUpdate(
          expectedRowCount,
          "INSERT INTO T (k, v) VALUES ('1-boo1', 1), ('2-boo2', 2), , ('1-boo1', 1) "
              + "ON CONFLICT(k) DO NOTHING");
    }
    assertThat(
            getClient(dialect.dialect)
                .singleUse(TimestampBound.strong())
                .readRow("T", Key.of("1-boo1"), Collections.singletonList("V"))
                .getLong(0))
        .isEqualTo(1);
    // Delete the rows
    executeUpdate(expectedRowCount, deleteDml());
    assertThat(
            getClient(dialect.dialect)
                .singleUse(TimestampBound.strong())
                .readRow("T", Key.of(String.format("%d-boo1", id)), Collections.singletonList("V")))
        .isNull();
  }

  @Test
  public void testInsertOrUpdateDuplicateRows() {
    try {
      if (dialect.dialect == Dialect.GOOGLE_STANDARD_SQL) {
        executeUpdate(
            0, "INSERT OR UPDATE INTO T (k, v) VALUES ('1-boo1', 1), ('2-boo2', 2), ('1-boo1', 1)");
      } else {
        executeUpdate(
            0,
            "INSERT INTO T (k, v) VALUES ('1-boo1', 1), ('2-boo2', 2), , ('1-boo1', 1) "
                + "ON CONFLICT(k) DO UPDATE SET k = expected.k, v = expected.v");
      }
    } catch (SpannerException e) {
      assertThat(e.getErrorCode()).isEqualTo(ErrorCode.INVALID_ARGUMENT);
      assertThat(e.getMessage()).contains("Cannot affect a row second time for key('1-boo1')");
    }

    // No rows to delete as insert was unsuccessful.
    executeUpdate(0, deleteDml());
    assertThat(
            getClient(dialect.dialect)
                .singleUse(TimestampBound.strong())
                .readRow("T", Key.of(String.format("%d-boo1", id)), Collections.singletonList("V")))
        .isNull();
  }

@arpan14 arpan14 added the do not merge Indicates a pull request not ready for merge, due to either quality or timing. label Jan 18, 2024
@product-auto-label product-auto-label bot added size: l Pull request size is large. and removed size: m Pull request size is medium. labels Jan 18, 2024
} else {
executeUpdate(
0,
"INSERT INTO T (k, v) VALUES ('1-boo1', 1), ('2-boo2', 2), , ('1-boo1', 1) "
Copy link

@shreyaprg shreyaprg Jan 19, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see a syntax error in this line because of an extra comma: INSERT INTO T (k, v) VALUES ('1-boo1', 1), ('2-boo2', 2), **, (**'1-boo1', 1)
Pls remove the extra comma
INSERT INTO T (k, v) VALUES ('1-boo1', 1), ('2-boo2', 2), ('1-boo1', 1)

} else {
executeUpdate(
expectedRowCount,
"INSERT INTO T (k, v) VALUES ('1-boo1', 1), ('2-boo2', 2), , ('1-boo1', 1) "

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see a syntax error in this line because of an extra comma: INSERT INTO T (k, v) VALUES ('1-boo1', 1), ('2-boo2', 2), , ('1-boo1', 1)
Pls remove the extra comma.
INSERT INTO T (k, v) VALUES ('1-boo1', 1), ('2-boo2', 2), ('1-boo1', 1)

@arpan14 arpan14 closed this Apr 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api: spanner Issues related to the googleapis/java-spanner API. do not merge Indicates a pull request not ready for merge, due to either quality or timing. size: l Pull request size is large.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants