-
Notifications
You must be signed in to change notification settings - Fork 1
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
add: edge id in SQL #80
Conversation
WalkthroughThe updates involve a significant reconfiguration of the edge table schema, introducing a unique identifier for edges, refining how edge and vertex IDs are managed, and adjusting related code and tests to align with the new schema. These changes streamline the database's structure and improve query handling, while also ensuring backward compatibility. Changes
Related issues
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
Codecov ReportAttention:
❗ Your organization needs to install the Codecov GitHub app to enable full functionality. Additional details and impacted files@@ Coverage Diff @@
## main #80 +/- ##
==========================================
+ Coverage 76.14% 76.27% +0.12%
==========================================
Files 19 19
Lines 327 333 +6
Branches 36 36
==========================================
+ Hits 249 254 +5
- Misses 78 79 +1 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Review Status
Actionable comments generated: 1
Configuration used: CodeRabbit UI
Files selected for processing (6)
- src/main/resources/application.conf (1 hunks)
- src/main/scala/domain/graph/GraphEdge.scala (3 hunks)
- src/main/scala/domain/table/ddl/column/ColumnType.scala (3 hunks)
- src/test/scala/domain/graph/GraphEdgeSpec.scala (3 hunks)
- src/test/scala/usecase/ByExhaustiveSearchSpec.scala (2 hunks)
- src/test/scala/usecase/UsingSpecificKeyListSpec.scala (4 hunks)
Additional comments: 11
src/main/resources/application.conf (1)
- 30-30: The addition of
column_name_edge_id
is noted and aligns with the summary's mention of a new column for edge IDs.src/test/scala/domain/graph/GraphEdgeSpec.scala (2)
- 30-32: Renaming of columns in the
edge
table fromin_v_id
andout_v_id
toid_in_v
andid_out_v
is consistent with the summary.- 57-58: The reordering of properties in the
RecordValue
to placeproperty_weight
beforelabel_knows
is consistent with the summary.src/main/scala/domain/graph/GraphEdge.scala (3)
- 22-22: The addition of a private
id
field in theGraphEdge
class is consistent with the summary's mention of a new field to handle edge IDs.- 31-34: The
toDdl
method has been updated to include theid
column in the generated table information, aligning with the summary.- 83-84: The
toDml
method now directly uses theid
field, which is an improvement over the previous version that calledvalue.id()
multiple times.src/test/scala/usecase/UsingSpecificKeyListSpec.scala (1)
- 39-43: The renaming of columns in the
vertex
table and the update of record values are consistent with the summary.src/test/scala/usecase/ByExhaustiveSearchSpec.scala (2)
- 101-108: The updates to column names and types within the
edge
table are consistent with the summary.- 118-167: The record values have been updated to reflect the changes in column names and types, aligning with the summary.
src/main/scala/domain/table/ddl/column/ColumnType.scala (2)
- 3-4: The import statement for
org.janusgraph.graphdb.relations.RelationIdentifier
is necessary for the new handling ofRelationIdentifier
types.- 88-89: The
ColumnType
object has been modified to handleRelationIdentifier
by converting it to a string, which is a sensible approach for database operations.
src/main/resources/application.conf
Outdated
column_name_edge_id="id" | ||
column_name_edge_in_v_id=${?COLUMN_NAME_EDGE_IN_V_ID} | ||
column_name_edge_out_v_id="out_v_id" | ||
column_name_edge_in_v_id="id_in_v" | ||
column_name_edge_in_v_id=${?COLUMN_NAME_EDGE_IN_V_ID} | ||
column_name_edge_out_v_id="id_out_v" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The environment variable for column_name_edge_in_v_id
is being set twice (lines 32 and 33), which is likely an error. The second assignment should probably be for a different variable.
- column_name_edge_in_v_id=${?COLUMN_NAME_EDGE_IN_V_ID}
+ column_name_edge_out_v_id=${?COLUMN_NAME_EDGE_OUT_V_ID}
Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
column_name_edge_id="id" | |
column_name_edge_in_v_id=${?COLUMN_NAME_EDGE_IN_V_ID} | |
column_name_edge_out_v_id="out_v_id" | |
column_name_edge_in_v_id="id_in_v" | |
column_name_edge_in_v_id=${?COLUMN_NAME_EDGE_IN_V_ID} | |
column_name_edge_out_v_id="id_out_v" | |
column_name_edge_id="id" | |
column_name_edge_in_v_id=${?COLUMN_NAME_EDGE_IN_V_ID} | |
column_name_edge_in_v_id="id_in_v" | |
column_name_edge_out_v_id=${?COLUMN_NAME_EDGE_OUT_V_ID} | |
column_name_edge_out_v_id="id_out_v" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files selected for processing (1)
- src/test/scala/usecase/ByExhaustiveSearchSpec.scala (2 hunks)
Files skipped from review as they are similar to previous changes (1)
- src/test/scala/usecase/ByExhaustiveSearchSpec.scala
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files selected for processing (1)
- src/main/resources/application.conf (1 hunks)
Files skipped from review as they are similar to previous changes (1)
- src/main/resources/application.conf
Summary by CodeRabbit
New Features
Bug Fixes
Refactor
Tests