fix: preserve SQL aliases in gRPC ResultSet serialization#2858
fix: preserve SQL aliases in gRPC ResultSet serialization#2858
Conversation
Summary of ChangesHello @robfrank, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request resolves a critical bug where SQL column aliases were not preserved during gRPC ResultSet serialization. The core solution involves refactoring the gRPC server's serialization logic to process Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
🧪 CI InsightsHere's what we observed from your CI run for 9734849. 🟢 All jobs passed!But CI Insights is watching 👀 |
There was a problem hiding this comment.
Code Review
This pull request successfully addresses a critical bug in the gRPC server where SQL column aliases were not being preserved in query results. The core of the fix is a shift from Record-level to Result-level serialization, which is a solid approach that aligns the gRPC behavior with the existing HTTP handler. The changes are well-supported by new and updated tests, and the addition of a detailed markdown file explaining the fix is excellent. I've identified a few minor issues, including a duplicated dependency in the pom.xml, a placeholder test name, and some redundant code in the new tests that could be cleaned up.
grpc-client/src/test/java/com/arcadedb/remote/grpc/GrpcServerPluginIT.java
Outdated
Show resolved
Hide resolved
grpc-client/src/test/java/com/arcadedb/remote/grpc/GrpcServerPluginIT.java
Outdated
Show resolved
Hide resolved
grpcw/src/main/java/com/arcadedb/server/grpc/ArcadeDbGrpcService.java
Outdated
Show resolved
Hide resolved
grpcw/src/main/java/com/arcadedb/server/grpc/ArcadeDbGrpcService.java
Outdated
Show resolved
Hide resolved
Coverage summary from CodacySee diff coverage on Codacy
Coverage variation details
Coverage variation is the difference between the coverage for the head and common ancestor commits of the pull request branch: Diff coverage details
Diff coverage is the percentage of lines that are covered by tests out of the coverable lines that the pull request added or modified: See your quality gate settings Change summary preferences |
bd03b1e to
9734849
Compare
This pull request addresses issue #2854 by fixing a critical bug in the gRPC server that prevented SQL column aliases from being preserved in query results. The main change is a switch from Record-level to Result-level serialization in the gRPC server, ensuring that aliased columns and metadata properties are correctly included in the response, matching the behavior of the HTTP handler. Comprehensive regression tests have been added and enabled to verify the fix, and related test dependencies are updated.
gRPC Server Serialization Improvements
ArcadeDbGrpcService.executeQuery()to use a newconvertResultToGrpcRecord()method that serializes fromResultobjects, allowing SQL aliases to be preserved in gRPC responses.@rid,@type) into the properties map for element-backed results to work around client-side limitations and ensure consistent metadata availability.Testing Enhancements
sqlAliasesArePreservedInGrpcResultSet()inRemoteGrpcDatabaseRegressionTest.javato verify that both original and aliased properties, as well as metadata, are present in gRPC query results. [1] [2]@Disabledfrom all existing tests inRemoteGrpcDatabaseRegressionTest.javaand refactored the test class to extendBaseGraphServerTest, enabling full integration testing with an embedded server. [1] [2] [3] [4] [5] [6]GrpcServerPluginITto further verify gRPC plugin behavior with real data and aliases.Build and Dependency Updates
grpc-client/pom.xmlto include test-scoped dependencies onarcadedb-grpcwandarcadedb-server, ensuring all necessary components are available for integration testing.Documentation
2854-grpc-resultset-alias.mdwith a detailed description of the bug, analysis, solution, implementation steps, and verification process.