mattcasters commented on code in PR #9200:
URL: https://github.com/apache/hop/pull/9200#discussion_r4435529587
##########
docs/hop-user-manual/modules/ROOT/pages/workflow/actions/graph-index.adoc:
##########
@@ -93,4 +93,19 @@ shows the exact `CREATE INDEX` or `DROP INDEX` statements
before execution.
The preview is read-only and helps you verify the index operations that will
be performed.
+== Result
+What this action passes on to the next action. See
xref:workflow/action-results.adoc[Action results] for how results travel
through a workflow.
+
+[options="header", width="90%", cols="1,3"]
+|===
+|Result|Value
+|True|All the index updates ran, with 0 errors. The drops run first, then the
creates. An empty list is also true, and so is an empty *Index name*: the
database names the index.
Review Comment:
**[bug]** An empty *Index name* is not always a true result. On Neo4j,
`CypherGraphDialect.getDropIndexStatement` throws before any statement runs
when the name is empty (`Please drop indexes with the name of the index`), and
`getCreateVectorIndexStatement` throws for a vector index with no name.
Memgraph vector indexes also require a name (`Memgraph vector indexes need a
name`). Apache AGE refuses an empty name for both create and drop (`Apache AGE
indexes need a name`), which this page already says in the database table
("Nodes, with a name"). In those cases `execute()` throws, the workflow stops,
and no hop is followed. An empty name succeeds only for a Neo4j range create
(the server assigns a name) and for Memgraph or FalkorDB range indexes, which
are not named.
**Suggestion:** Limit the true case to creating a Neo4j range index, or a
Memgraph/FalkorDB range index. State that a drop, a Neo4j or Memgraph vector
index, or an Apache AGE index with an empty name stops the workflow.
##########
docs/hop-user-manual/modules/ROOT/pages/workflow/actions/graph-index.adoc:
##########
@@ -93,4 +93,19 @@ shows the exact `CREATE INDEX` or `DROP INDEX` statements
before execution.
The preview is read-only and helps you verify the index operations that will
be performed.
+== Result
+What this action passes on to the next action. See
xref:workflow/action-results.adoc[Action results] for how results travel
through a workflow.
+
+[options="header", width="90%", cols="1,3"]
+|===
+|Result|Value
+|True|All the index updates ran, with 0 errors. The drops run first, then the
creates. An empty list is also true, and so is an empty *Index name*: the
database names the index.
+|False|The action is never false: see the WARNING.
+|Result files|Passed on unchanged.
+|Result rows|Passed on unchanged.
+|===
+
+WARNING: When an index update can't be done, the workflow stops with an error
and no hop from the action is followed: the *Graph database connection* is
empty, doesn't exist or can't be reached, an update has no *Update type*, a
VECTOR index has no *Vector dimensions*, or the database refuses the statement.
The updates before it stay done, the ones after it are not run.
Review Comment:
**[suggestion]** "a VECTOR index has no *Vector dimensions*" stops the
workflow only when the update is a create. A drop builds `DROP INDEX <name> IF
EXISTS` and never reads the dimensions (`toVectorIndexDefinition(...,
creating=false)`). A create also stops when the value is not a positive
integer, not only when it is empty (`parsePositive`). A missing object name or
property list throws the same way (`Please specify the properties to index`)
and is neither a false result nor "the database refuses the statement".
**Suggestion:** Say that creating a VECTOR index with empty, non-numeric, or
non-positive *Vector dimensions* stops the workflow, and that a missing label
or property list does too. A drop does not need dimensions.
##########
docs/hop-user-manual/modules/ROOT/pages/workflow/actions/graph-constraint.adoc:
##########
@@ -56,4 +56,19 @@ shows the exact `CREATE CONSTRAINT` or `DROP CONSTRAINT`
statements before execu
The preview is read-only and helps you verify the constraint operations that
will be performed,
including the correct syntax for UNIQUE, NOT_NULL, and NODE_KEY constraints.
+== Result
+What this action passes on to the next action. See
xref:workflow/action-results.adoc[Action results] for how results travel
through a workflow.
+
+[options="header", width="90%", cols="1,3"]
+|===
+|Result|Value
+|True|All the constraint updates ran, with 0 errors. The drops run first, then
the creates. An empty list is also true.
+|False|The action is never false: see the WARNING.
+|Result files|Passed on unchanged.
+|Result rows|Passed on unchanged.
+|===
+
+WARNING: When a constraint update can't be done, the workflow stops with an
error and no hop from the action is followed: the *Graph database connection*
is empty, doesn't exist or can't be reached, *Constraint name* is empty, or the
database refuses the statement, for example NOT_NULL or NODE_KEY on Neo4j
Community Edition, or a UNIQUE constraint on data that already has duplicate
values. The updates before it stay done, the ones after it are not run.
Review Comment:
**[suggestion]** A blank *Update type* throws `Please make sure to always
specify a constraint update type` and stops the workflow, the same case the
Graph index page documents, but it is not in this list. An empty *Constraint
name* stops the workflow only on Neo4j (`Please create/drop constraints with a
name`). Memgraph constraints are identified by label and property (`CONSTRAINT
ON (n:Label) ASSERT ...`), so an empty name does not abort. An empty *Object
property* stops a create (`constraint requires at least one property`) but not
a drop, which only uses the name.
**Suggestion:** Add a missing *Update type*, and qualify the empty
*Constraint name* and empty *Object property* cases as Neo4j (the property only
on create).
##########
docs/hop-user-manual/modules/ROOT/pages/workflow/actions/graph-constraint.adoc:
##########
@@ -56,4 +56,19 @@ shows the exact `CREATE CONSTRAINT` or `DROP CONSTRAINT`
statements before execu
The preview is read-only and helps you verify the constraint operations that
will be performed,
including the correct syntax for UNIQUE, NOT_NULL, and NODE_KEY constraints.
+== Result
+What this action passes on to the next action. See
xref:workflow/action-results.adoc[Action results] for how results travel
through a workflow.
+
+[options="header", width="90%", cols="1,3"]
+|===
+|Result|Value
+|True|All the constraint updates ran, with 0 errors. The drops run first, then
the creates. An empty list is also true.
+|False|The action is never false: see the WARNING.
+|Result files|Passed on unchanged.
+|Result rows|Passed on unchanged.
+|===
+
+WARNING: When a constraint update can't be done, the workflow stops with an
error and no hop from the action is followed: the *Graph database connection*
is empty, doesn't exist or can't be reached, *Constraint name* is empty, or the
database refuses the statement, for example NOT_NULL or NODE_KEY on Neo4j
Community Edition, or a UNIQUE constraint on data that already has duplicate
values. The updates before it stay done, the ones after it are not run.
+
+NOTE: The action creates with `IF NOT EXISTS` and drops with `IF EXISTS`.
Creating a constraint that already exists, or dropping one that doesn't, does
nothing and the action is still true. On Neo4j this is also the case when a
constraint with the same name exists on another label or property, or an
equivalent one exists with another name: nothing is created.
Review Comment:
**[suggestion]** Memgraph constraint statements have no `IF NOT EXISTS` /
`IF EXISTS` (`CREATE CONSTRAINT ON (n:Label) ASSERT ...`). The page description
already includes Memgraph. The "nothing is created" outcome for an equivalent
constraint with another name matches Neo4j's `IF NOT EXISTS`, not Memgraph. An
existing Memgraph constraint stays true only when the error text is swallowed
by `isExistingOrMissingIndexError`.
**Suggestion:** Start with "On Neo4j the action creates with `IF NOT EXISTS`
and drops with `IF EXISTS`."
##########
docs/hop-user-manual/modules/ROOT/pages/workflow/actions/graph-index.adoc:
##########
@@ -93,4 +93,19 @@ shows the exact `CREATE INDEX` or `DROP INDEX` statements
before execution.
The preview is read-only and helps you verify the index operations that will
be performed.
+== Result
+What this action passes on to the next action. See
xref:workflow/action-results.adoc[Action results] for how results travel
through a workflow.
+
+[options="header", width="90%", cols="1,3"]
+|===
+|Result|Value
+|True|All the index updates ran, with 0 errors. The drops run first, then the
creates. An empty list is also true, and so is an empty *Index name*: the
database names the index.
+|False|The action is never false: see the WARNING.
+|Result files|Passed on unchanged.
+|Result rows|Passed on unchanged.
+|===
+
+WARNING: When an index update can't be done, the workflow stops with an error
and no hop from the action is followed: the *Graph database connection* is
empty, doesn't exist or can't be reached, an update has no *Update type*, a
VECTOR index has no *Vector dimensions*, or the database refuses the statement.
The updates before it stay done, the ones after it are not run.
+
+NOTE: The action creates with `IF NOT EXISTS` and drops with `IF EXISTS`.
Creating an index that already exists, or dropping one that doesn't, does
nothing and the action is still true. On Neo4j this is also the case when an
index with the same name exists on another label or property, or an equivalent
one exists with another name: nothing is created.
Review Comment:
**[suggestion]** `IF NOT EXISTS` / `IF EXISTS` is the Neo4j and Apache AGE
wording. Memgraph emits `CREATE INDEX ON :Label(props)` and `DROP INDEX ON
:Label(props)`, and FalkorDB emits `CREATE INDEX FOR ...` / `DROP INDEX FOR
...`, with no `IF NOT EXISTS`. On those databases an existing index stays a
true result only when the driver error is treated as already present
(`isExistingOrMissingIndexError`), which is a different mechanism and was not
part of the Neo4j test run. This page's database table includes all four.
**Suggestion:** Open the note with "On Neo4j and Apache AGE the action
creates with `IF NOT EXISTS` and drops with `IF EXISTS`." Keep the same-name /
equivalent-index sentence Neo4j-only, as it already is.
--
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]
