Skip to content
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

feat: Add parser for schemarb #87

Merged
merged 16 commits into from
Nov 19, 2024
Merged

feat: Add parser for schemarb #87

merged 16 commits into from
Nov 19, 2024

Conversation

sasamuku
Copy link
Member

Summary

This is proof of concept for parsing schema.rb to DBStructure.

Related Issue

N/A

Changes

  • ab138d3
    • fetch grammar from DBML
    • create parser.js by npx peggy parser.pegjs
  • 8939685
    • add parser.ts used for entrypoint
    • add tests to check if convert correctly

Testing

Other Information

Comment on lines 2 to 3
import { relationshipsSchema, tableNameSchema, tableSchema } from './database'
import { nodeMetaDataSchema } from './erd'
Copy link
Member Author

Choose a reason for hiding this comment

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

there are two structure types, database and erd, so I divide structure into each file.
If this poc is good enough to merge, I am going to fix other packages where ci is failed.

@sasamuku sasamuku marked this pull request as ready for review November 18, 2024 03:29
@sasamuku sasamuku requested a review from a team as a code owner November 18, 2024 03:29
@sasamuku sasamuku requested review from hoshinotsuyoshi, FunamaYukina, junkisai and MH4GF and removed request for a team November 18, 2024 03:29
relationships: relationshipsSchema,
})

export type DBStructure = v.InferOutput<typeof dbStructureSchema>
export type ERDStructure = v.InferOutput<typeof erdStructureSchema>
Copy link
Member Author

Choose a reason for hiding this comment

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

ERD has a metadata such as node position and colors.
As schema name, ERDStructure is better than DBStructure.
Because DBStructure doesn't seem to have any data related to erd.
How do you think?

Copy link
Member

Choose a reason for hiding this comment

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

I agree that the parser wants to remain ignorant of information about the UI of the ER Diagrams.
I think we should leave DBStructure as it is for this PR right now, since we can consider the schema when implementing ERD!


type SupportedFormat = 'schemarb' | 'postgres'

const convertToDBStructure = (data: any): DBStructure => {
Copy link
Member Author

@sasamuku sasamuku Nov 18, 2024

Choose a reason for hiding this comment

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

convertToDBStructure should not be implemented.
It would be better Generated parser.js to return directly DBStructure.
So I might fix pegjs for next step if this PR is merged.

acc[table.name] = {
comment: null,
fields: table.fields.map((field: any) => ({
check: null,
Copy link
Member Author

Choose a reason for hiding this comment

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

there is a lack of fields such as unique and check.
This is because fields parsed by pegjs do not contain these data.

This is also issue to solve by editing pegjs.

version = "version"
do = "do"
end = "end"
lambda_function "lambda function" = "=>" / "->"
Copy link
Member

Choose a reason for hiding this comment

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

In Ruby, -> is used as a shorthand for lambda, while => is typically used to define key-value pairs in a Hash.

Copy link
Member

Choose a reason for hiding this comment

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

This was fixed in 8d86366, but => might still appear, so handling => itself might still be necessary. That said, we could also address it if and when any issues arise. Let’s proceed with this for now!

frontend/packages/db-structure/biome.jsonc Outdated Show resolved Hide resolved
frontend/packages/db-structure/package.json Outdated Show resolved Hide resolved
@@ -7,14 +7,18 @@
"lint:biome": "biome check .",
"lint:tsc": "tsc --noEmit",
"fmt": "conc -c auto pnpm:fmt:*",
"fmt:biome": "biome check --write --unsafe ."
"fmt:biome": "biome check --write --unsafe .",
"test": "vitest"
Copy link
Member

Choose a reason for hiding this comment

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

good 👍🏻

Copy link
Member

Choose a reason for hiding this comment

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

👍

@sasamuku
Copy link
Member Author

Fixed:

  • add license comment in parser.pegjs
  • lodash fucntions are separately installed

Next work:

  • fix pegjs to fit our schema (tried in this pr but needs more time)
  • add types to not use Any

// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.
Copy link
Member

Choose a reason for hiding this comment

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

Thank you for adding it! 😄

@sasamuku
Copy link
Member Author

sasamuku commented Nov 18, 2024

add license comment in parser.pegjs

I refereed to https://github.com/holistics/dbml/blob/master/LICENSE

  1. Redistribution. You may reproduce and distribute copies of the
    Work or Derivative Works thereof in any medium, with or without
    modifications, and in Source or Object form, provided that You
    meet the following conditions:
    (a) You must give any other recipients of the Work or
    Derivative Works a copy of this License; and
    (b) You must cause any modified files to carry prominent notices
    stating that You changed the files; and
    (c) You must retain, in the Source form of any Derivative Works
    that You distribute, all copyright, patent, trademark, and
    attribution notices from the Source form of the Work,
    excluding those notices that do not pertain to any part of
    the Derivative Works; and
    (d) If the Work includes a "NOTICE" text file as part of its
    distribution, then any Derivative Works that You distribute must
    include a readable copy of the attribution notices contained
    within such NOTICE file, excluding those notices that do not
    pertain to any part of the Derivative Works, in at least one
    of the following places: within a NOTICE text file distributed
    as part of the Derivative Works; within the Source form or
    documentation, if provided along with the Derivative Works; or,
    within a display generated by the Derivative Works, if and
    wherever such third-party notices normally appear. The contents
    of the NOTICE file are for informational purposes only and
    do not modify the License. You may add Your own attribution
    notices within Derivative Works that You distribute, alongside
    or as an addendum to the NOTICE text from the Work, provided
    that such additional attribution notices cannot be construed
    as modifying the License.

sample (MIT not Apache2.0): https://github.com/antlr/grammars-v4/blob/master/sql/postgresql/PostgreSQLLexer.g4

Copy link
Member

@hoshinotsuyoshi hoshinotsuyoshi left a comment

Choose a reason for hiding this comment

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

looks good to me!

Copy link
Member

@MH4GF MH4GF left a comment

Choose a reason for hiding this comment

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

LGTM 🚀🚀🚀

@sasamuku sasamuku added this pull request to the merge queue Nov 19, 2024
Merged via the queue into main with commit 71ef4cd Nov 19, 2024
8 checks passed
@sasamuku sasamuku deleted the add_parser_for_schemarb branch November 19, 2024 01:00
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.

3 participants