mattcasters commented on code in PR #8849:
URL: https://github.com/apache/hop/pull/8849#discussion_r4237913796
##########
plugins/tech/age/src/main/java/org/apache/hop/age/AgeGraphDialect.java:
##########
@@ -45,6 +50,82 @@ public String toString() {
return getId();
}
+ @Override
+ public boolean isSupportingNodeIndexes() {
+ return true;
+ }
+
+ /**
+ * Create the label if it doesn't exist yet, then the index on its table:
the index can be created
+ * before the nodes are loaded.
+ */
+ @Override
+ public String getCreateIndexStatement(GraphIndexDefinition index) throws
HopException {
+ validateIndex(index);
+ if (index.properties().isEmpty()) {
+ throw new HopException("Please specify the properties to index on " +
index.objectName());
+ }
+ return getCreateLabelStatement(index.objectName())
+ + ";\n"
+ + getCreateNodeIndexStatement(index.name(), index.objectName(),
index.properties());
+ }
+
+ @Override
+ public String getDropIndexStatement(GraphIndexDefinition index) throws
HopException {
+ validateIndex(index);
+ return "DROP INDEX IF EXISTS "
+ + quoteIdentifier(graphName)
+ + '.'
+ + quoteIdentifier(index.name());
+ }
+
+ /** Only named indexes on nodes: PostgreSQL needs the name to create the
index if not exists. */
+ private void validateIndex(GraphIndexDefinition index) throws HopException {
+ if (index.isRelationship()) {
+ throw IGraphDialect.indexesNotSupported(this, index.objectType(),
index.objectName());
+ }
+ if (StringUtils.isEmpty(index.objectName())) {
+ throw new HopException("Please specify the label of the index " +
index.name());
+ }
+ if (StringUtils.isEmpty(index.name())) {
+ throw new HopException(
+ "Apache AGE indexes need a name. Label: "
+ + index.objectName()
+ + ", properties: "
+ + String.join(",", index.properties()));
+ }
+ }
+
+ /**
+ * A block creating the node label in the graph unless it exists: AGE
creates the table of a label
+ * with the first node.
+ */
+ String getCreateLabelStatement(String label) {
+ String graphLiteral = quoteLiteral(graphName);
+ String labelLiteral = quoteLiteral(label);
+ String body =
+ "BEGIN IF NOT EXISTS (SELECT 1 FROM ag_catalog.ag_label l"
+ + " JOIN ag_catalog.ag_graph g ON g.graphid = l.graph WHERE g.name
= "
+ + graphLiteral
+ + " AND l.kind = 'v' AND l.name = "
+ + labelLiteral
+ + ") THEN PERFORM ag_catalog.create_vlabel("
Review Comment:
**[suggestion]** `getCreateLabelStatement` checks `ag_label` and then calls
`create_vlabel`, which errors with "label already exists" instead of no-opping.
That check and the call are not atomic, and both the `DO` block and the `CREATE
INDEX` are one `executeSchemaStatement` call. If two sessions create the same
new label, the loser fails inside the `DO` block; pgjdbc then does not send the
`CREATE INDEX`, and `AgeGraphDialect` does not treat "already exists" as
`isExistingOrMissingIndexError`. The index action fails even though a retry
would succeed. `AgeGraphDatabase.createGraphIfMissing` already catches this
race for `create_graph` and re-checks the catalog.
**Suggestion:** On "already exists", re-check `ag_label` and continue with
`CREATE INDEX IF NOT EXISTS`, or split the two statements so a duplicate-label
error does not skip the index. `CREATE INDEX IF NOT EXISTS` is already safe to
repeat.
--
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]
