Skip to content

Conversation

@nktks
Copy link
Owner

@nktks nktks commented Mar 14, 2025

This PR is created by Cline.

This PR replaces the spansql dependency with memefish v0.4.0. The main changes are:

  1. Updated the go.mod file to add the memefish v0.4.0 dependency and remove the cloud.google.com/go/spanner dependency.
  2. Modified the cli.go file to use memefish parser instead of spansql parser.
  3. Updated the graph.go file to use memefish AST types instead of spansql types.

The parser now correctly handles SQL statements separated by semicolons, which fixes the issue with empty ER diagrams.

@nktks nktks requested a review from Copilot March 14, 2025 07:24
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR replaces the spansql dependency with memefish v0.4.0, updating the parsing and graph generation logic accordingly.

  • Dependency update and import modifications in cli.go and graph.go
  • Updated SQL parsing in cli.go to handle semicolon-separated statements
  • Adjusted graph generation logic in graph.go to work with memefish AST types

Reviewed Changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.

File Description
cli.go Replaces spansql with memefish, updates parsing logic and adds detailed logging, and improves dot command error handling
graph.go Updates functions to use memefish AST types and refines table node and foreign key edge handling
Comments suppressed due to low confidence (2)

cli.go:132

  • Parsing errors are currently only logged and then skipped, which may lead to silently ignoring failed SQL statements. Consider returning an error or accumulating errors to make the failure more explicit.
if err != nil { log.Printf("Error parsing statement: %v", err) continue }

graph.go:120

  • [nitpick] The comment above refers to 'Handle interleave' even though the code uses the 'Cluster' field; updating the comment to reflect the correct field may improve clarity.
if t.Cluster != nil {

@nktks nktks merged commit f3d3afc into master Mar 14, 2025
3 checks passed
@nktks nktks deleted the use-memefish branch March 14, 2025 07:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants