amogh-jahagirdar commented on code in PR #8340:
URL: https://github.com/apache/iceberg/pull/8340#discussion_r1419451962
##########
core/src/test/java/org/apache/iceberg/jdbc/TestJdbcCatalog.java:
##########
@@ -601,15 +601,76 @@ public void testDropNamespace() {
public void testCreateNamespace() {
Namespace testNamespace = Namespace.of("testDb", "ns1", "ns2");
assertThat(catalog.namespaceExists(testNamespace)).isFalse();
- // Test with no metadata
+ assertThat(catalog.namespaceExists(Namespace.of("testDb",
"ns1"))).isFalse();
catalog.createNamespace(testNamespace);
assertThat(catalog.namespaceExists(testNamespace)).isTrue();
+ assertThat(catalog.namespaceExists(Namespace.of("testDb"))).isTrue();
+ assertThat(catalog.namespaceExists(Namespace.of("testDb",
"ns1"))).isTrue();
+ assertThat(catalog.namespaceExists(Namespace.of("ns1", "ns2"))).isFalse();
+ assertThat(catalog.namespaceExists(Namespace.of("testDb",
"ns%"))).isFalse();
+ assertThat(catalog.namespaceExists(Namespace.of("testDb",
"ns_"))).isFalse();
+ assertThat(catalog.namespaceExists(Namespace.of("testDb", "ns1",
"ns2"))).isTrue();
+ assertThat(catalog.namespaceExists(Namespace.of("testDb", "ns1", "ns2",
"ns3"))).isFalse();
+ }
+
+ @Test
+ public void testCreateNamespaceWithBackslashCharacter() {
+
+ Namespace testNamespace = Namespace.of("test\\Db", "ns\\1", "ns3");
+ assertThat(catalog.namespaceExists(testNamespace)).isFalse();
+ catalog.createNamespace(testNamespace);
+ assertThat(catalog.namespaceExists(testNamespace)).isTrue();
+ assertThat(catalog.namespaceExists(Namespace.of("test\\Db",
"ns\\1"))).isTrue();
+ //
+ assertThat(catalog.namespaceExists(Namespace.of("test\\%Db",
"ns\\.1"))).isFalse();
+ assertThat(catalog.namespaceExists(Namespace.of("test%Db",
"ns\\.1"))).isFalse();
+ assertThat(catalog.namespaceExists(Namespace.of("test%Db"))).isFalse();
+ assertThat(catalog.namespaceExists(Namespace.of("test%"))).isFalse();
+ assertThat(catalog.namespaceExists(Namespace.of("test\\%"))).isFalse();
+ assertThat(catalog.namespaceExists(Namespace.of("test_Db",
"ns\\.1"))).isFalse();
+ //
+ testNamespace = Namespace.of("test\\%Db2", "ns1");
+ assertThat(catalog.namespaceExists(testNamespace)).isFalse();
+ catalog.createNamespace(testNamespace);
+ assertThat(catalog.namespaceExists(testNamespace)).isTrue();
+ assertThat(catalog.namespaceExists(Namespace.of("test\\%Db2"))).isTrue();
+ assertThat(catalog.namespaceExists(Namespace.of("test%Db2"))).isFalse();
+ assertThat(catalog.namespaceExists(Namespace.of("test\\_Db2"))).isFalse();
+ }
+
+ @Test
+ public void testCreateNamespaceWithPercentCharacter() {
+
Review Comment:
Same as @nastra pointed out before, unnecessary new line after the method
starts.
##########
core/src/test/java/org/apache/iceberg/jdbc/TestJdbcCatalog.java:
##########
@@ -601,15 +601,76 @@ public void testDropNamespace() {
public void testCreateNamespace() {
Namespace testNamespace = Namespace.of("testDb", "ns1", "ns2");
assertThat(catalog.namespaceExists(testNamespace)).isFalse();
- // Test with no metadata
+ assertThat(catalog.namespaceExists(Namespace.of("testDb",
"ns1"))).isFalse();
catalog.createNamespace(testNamespace);
assertThat(catalog.namespaceExists(testNamespace)).isTrue();
+ assertThat(catalog.namespaceExists(Namespace.of("testDb"))).isTrue();
+ assertThat(catalog.namespaceExists(Namespace.of("testDb",
"ns1"))).isTrue();
+ assertThat(catalog.namespaceExists(Namespace.of("ns1", "ns2"))).isFalse();
+ assertThat(catalog.namespaceExists(Namespace.of("testDb",
"ns%"))).isFalse();
+ assertThat(catalog.namespaceExists(Namespace.of("testDb",
"ns_"))).isFalse();
+ assertThat(catalog.namespaceExists(Namespace.of("testDb", "ns1",
"ns2"))).isTrue();
+ assertThat(catalog.namespaceExists(Namespace.of("testDb", "ns1", "ns2",
"ns3"))).isFalse();
Review Comment:
Just a nit: I think I'd find it easier to read if all the "true" assertions
are grouped together, and then if all the "false" assertions are grouped
together. It makes it easier to verify the expected behavior at a glance. But
up to you, I see this still covers the expected cases.
##########
core/src/test/java/org/apache/iceberg/jdbc/TestJdbcCatalog.java:
##########
@@ -601,15 +601,76 @@ public void testDropNamespace() {
public void testCreateNamespace() {
Namespace testNamespace = Namespace.of("testDb", "ns1", "ns2");
assertThat(catalog.namespaceExists(testNamespace)).isFalse();
- // Test with no metadata
+ assertThat(catalog.namespaceExists(Namespace.of("testDb",
"ns1"))).isFalse();
catalog.createNamespace(testNamespace);
assertThat(catalog.namespaceExists(testNamespace)).isTrue();
+ assertThat(catalog.namespaceExists(Namespace.of("testDb"))).isTrue();
+ assertThat(catalog.namespaceExists(Namespace.of("testDb",
"ns1"))).isTrue();
+ assertThat(catalog.namespaceExists(Namespace.of("ns1", "ns2"))).isFalse();
+ assertThat(catalog.namespaceExists(Namespace.of("testDb",
"ns%"))).isFalse();
+ assertThat(catalog.namespaceExists(Namespace.of("testDb",
"ns_"))).isFalse();
+ assertThat(catalog.namespaceExists(Namespace.of("testDb", "ns1",
"ns2"))).isTrue();
+ assertThat(catalog.namespaceExists(Namespace.of("testDb", "ns1", "ns2",
"ns3"))).isFalse();
+ }
+
+ @Test
+ public void testCreateNamespaceWithBackslashCharacter() {
+
+ Namespace testNamespace = Namespace.of("test\\Db", "ns\\1", "ns3");
+ assertThat(catalog.namespaceExists(testNamespace)).isFalse();
+ catalog.createNamespace(testNamespace);
+ assertThat(catalog.namespaceExists(testNamespace)).isTrue();
+ assertThat(catalog.namespaceExists(Namespace.of("test\\Db",
"ns\\1"))).isTrue();
+ //
Review Comment:
Instead of "//" if we want to comment a deliniation could you comment with
some details? Or just remove it and add a new line separator between the two
namespaces which are being tested.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
