Skip to content

Update configuration structure to support multiple languages #302

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

Closed
kyleconroy opened this issue Jan 26, 2020 · 15 comments · Fixed by #318
Closed

Update configuration structure to support multiple languages #302

kyleconroy opened this issue Jan 26, 2020 · 15 comments · Fixed by #318
Labels
enhancement New feature or request

Comments

@kyleconroy
Copy link
Collaborator

The current version of the configuration file can't support multiple languages. Packages have Go-specific settings that do not apply to other languages.

@kyleconroy kyleconroy added the enhancement New feature or request label Jan 26, 2020
@kyleconroy kyleconroy mentioned this issue Jan 26, 2020
7 tasks
@kyleconroy
Copy link
Collaborator Author

Here's an example configuration file using all the existing options, sans picking an engine. Notice that Go-specific information is contained in each section.

{
  "version": "1",
  "packages": [
    {
      "name": "db",
      "path": "src/lib/db",
      "queries": "sql/db",
      "schema": "src/migrations",
      "emit_prepared_queries": true,
      "emit_json_tags": true,
      "overrides": [{
        "go_type": "github.com/foo/bar.Baz",
        "column": "foo"
      }]
    }
  ],
  "overrides": [{
    "go_type": "github.com/foo/bar.Baz",
    "postgres_type": "integer"
  }]
  "rename": {
    "sid": "SID"
  }
}

Here's a rough idea based on uber/prototool. I'm not sure what to do about global overrides or renaming support.

{
  "version": "1",
  "packages": [
    {
      "queries": "sql/db",
      "schema": "src/migrations",
      "plugins": {
        "go": {
          "name": "db",
          "path": "src/lib/db",
          "emit_prepared_queries": true,
          "emit_json_tags": true,
          "overrides": [{
            "go_type": "github.com/foo/bar.Baz",
            "column": "foo"
          }]
        }
      }
    }
  ]
}

@kyleconroy
Copy link
Collaborator Author

It's also getting complicated enough that we may want to switch to YAML or TOML

@mightyguava
Copy link
Contributor

mightyguava commented Jan 26, 2020

Switching to YAML for discussion:

version: 1
packages:
  - queries: sql/db
    schema: src/migrations
    plugins:
      go:
        name: db
        path: src/lib/db
        emit_prepared_queries: true
        emit_json_tags: true
        overrides:
          - go_type: github.com/foo/bar.Baz
            column: foo

So I guess also generating Kotlin output would look like the below? Changing go_type => gen_type and postgres_type => db_type to allow the names to be consistent across backends/versions. Add maybe we add globals, also by engine, since it's unlikely the same overrides and renames will apply across languages.

version: 2
packages:
  - queries: sql/db
    schema: src/migrations
    plugins:
      go:
        name: db
        out: src/lib/db
        emit_prepared_queries: true
        emit_json_tags: true
        overrides:
          - gen_type: github.com/foo/bar.Baz
            column: foo
      kotlin:
        name: com.example.db
        out: src/main/kotlin
        overrides:
          - gen_type: okio.ByteString
            db_type: ByteArray
global:
  go:
    overrides:
      - gen_type: github.com/foo/bar.Baz
        db_type: integer
    rename:
      - sid: SID
  kotlin:
    overrides:
      - gen_type: com.example.Foo
        db_type: jsonb

This seems to be evolving towards a single sqlc config file per repo, even for monorepos that have many languages and database engines. This approach makes sense for protobuf, which is built for data interchange across services. But I don't think it's the right one for sqlc.

Databases aren't meant to be shared by multiple applications. And a single application is unlikely to have 2 different SQL engines. So maybe let's constrain the problem such that a single sqlc config file will support a single application, with a single engine and a single language per config file. We can still have namespacing for language-specific configs:

For Go:

version: 2
lang: go
engine: postgres
packages:
  - name: db
    queries: sql/db
    schema: src/migrations
    out: src/lib/db
    overrides:
      - gen_type: github.com/foo/bar.Baz
        column: foo
    go:
      emit_prepared_queries: true
      emit_json_tags: true
global:
  overrides:
    - gen_type: github.com/foo/bar.Baz
      db_type: integer
  rename:
    - sid: SID

For Kotlin:

version: 2
lang: kotlin
engine: postgres
packages:
  - name: com.example.db
    queries: sql/db
    schema: src/migrations
    out: src/main/kotlin
    overrides:
      - gen_type: okio.ByteString
        db_type: ByteArray
global:
  overrides:
    - gen_type: com.example.Foo
      db_type: jsonb

I think supporting multiple packages in a single sqlc config still makes sense, to allow separation of concerns between different database entities for larger application.

And finally, if a user does have the situation where they have multiple services sharing the same db, in the same monorepo, we can still have a single file, since you could just put multiple yaml documents in the same file. sqlc could treat this as if it were multiple invocations of the tool.

---
version: 2
lang: go
engine: postgres
... configs for go ...
---
version: 2
lang: kotlin
engine: postgres
... configs for kotlin ...

@kyleconroy
Copy link
Collaborator Author

And a single application is unlikely to have 2 different SQL engines.

Sadly I haven't found this to be true. The last place I worked had a monorepo where packages were talking both to PosrtgreSQL and SQLite. I think the configuration file should be able to handle multiple engine types.

since you could just put multiple yaml documents in the same file

I find the multi-doc syntax for YAML files confusing, so I'd like to avoid using it.

I'm not sold on YAML as an alternative to JSON. Right now I'd like to focus on defining version 2 using JSON, and then picking a different, more human-friendly configuration format at a later date.

@mightyguava
Copy link
Contributor

mightyguava commented Jan 29, 2020

I just used yaml in my examples for sake of discussion, because it's too much of a PITA to type out so much JSON in text. Happy with JSON. I don't really have an opinion.

The last place I worked had a monorepo where packages were talking both to PosrtgreSQL and SQLite. I think the configuration file should be able to handle multiple engine types.

I still think this falls into the "unlikely" camp, but it is a totally reasonable use case yeah. If you don't want to do multi-doc syntax, maybe like this? (again, imagine this were JSON 😄 )

version: 2
lang: go
packages:
  - name: db
    engine: postgres
    queries: sql/queries
    schema: sql/migrations
    out: src/lib/db
    overrides:
      - gen_type: github.com/foo/bar.Baz
        column: text
    go:
      emit_prepared_queries: true
      emit_json_tags: true
  - name: config
    engine: sqlite
    queries: configdb/queries
    schema: configdb/migrations
    out: src/lib/configdb
    overrides:
      - gen_type: github.com/foo/bar.Baz
        column: varchar
global:
  postgres-overrides:
      - gen_type: github.com/foo/bar.Baz
        db_type: integer
  rename:
    - sid: SID

Just throwing ideas out here, feel free to scrap this.

While we are chatting about config, I have a separate nit: It's weird that keys are snake_case. In my experience, use of snake_case in JSON is tends to come from compatibility with legacy systems, like it was being used in Python, or just because someone saw it done that way elsewhere. It's generally preferred nowadays to use camelCase in JSON. E.g. protobuf 3 defaults to camel, and so does json api standard. Where json configs are common (in Javascript/Typescript world), keys are also always camelCase.

@kyleconroy
Copy link
Collaborator Author

It's generally preferred nowadays to use camelCase in JSON

Version 1 used snake_case, so I'm inclined to keep snake_case going forward for compatibility reasons. I don't think moving to a different case format is worth a breaking change.

@mightyguava
Copy link
Contributor

Yeah that was just a nit, sounds good. Regarding breaking changes to the config format though, I imagined that for config version upgrades, we would add a sqlc config-upgrade command to upgrade your config version and apply any breaking changes. I think we'd want an upgrade command regardless, since going forward engine should probably be a required field, some fields might get renamed, or moved, etc.

@kyleconroy
Copy link
Collaborator Author

kyleconroy commented Jan 30, 2020

Alright, here's my proposal for version two. It's a combination of a few of these proposals. It leaves the majority of the configuration the same, with a few small changes to accommodate multiple languages.

Here's the sqlc.json from the README ported to the syntax:

{
  "version": "2",
  "sql": [
    {
      "queries": "./sql/query/",
      "schema": "./sql/schema/",
      "engine": "postgresql",
      "gen": {
        "go": {
          "out": "internal/db",
          "package": "db",
          "emit_json_tags": true,
          "emit_prepared_queries": false,
          "emit_interface": true
        }
      }
    }
  ]
}

And here's the one of the examples above in the new syntax:

{
  "version": "2",
  "sql": [
    {
      "queries": "sql/db",
      "schema": "src/migrations",
      "engine": "postgresql",
      "gen": {
        "go": {
          "package": "db",
          "out": "src/lib/db",
          "emit_prepared_queries": true,
          "emit_json_tags": true,
          "overrides": [
            {
              "go_type": "github.com/foo/bar.Baz",
              "column": "foo"
            }
          ]
        },
        "kotlin": {
          "package": "com.example.db",
          "out": "src/main/kotlin",
          "overrides": [
            {
              "gen_type": "okio.ByteString",
              "db_type": "ByteArray"
            }
          ]
        }
      }
    }
  ],
  "gen": {
    "go": {
      "overrides": [
        {
          "go_type": "github.com/foo/bar.Baz",
          "db_type": "integer"
        }
      ],
      "rename": {
        "sid": "SID"
      }
    },
    "kotlin": {
      "overrides": [
        {
          "gen_type": "com.example.Foo",
          "db_type": "jsonb"
        }
      ]
    }
  }
}

For now, the only supported format will be JSON. We'll look to add YAML support soon, but I'd like that to land once version two is shipped. A few of the parameters were renamed to be more clear (path -> out, name -> package). Since the overrides are now scoped to a specific backend, we don't need to rename go_type to gen_type.

I've also ran this syntax past two sqlc users and they've given the thumbs up.

@eullerpereira94
Copy link
Contributor

Does this new syntax assumes that the default driver is the PostgreSQL one, or this still doesn't contemplate MySQL?

@kyleconroy
Copy link
Collaborator Author

kyleconroy commented Jan 30, 2020

For completeness, here's what the above examples will look like in YAML

version: '2'
sql:
- queries: "./sql/query/"
  schema: "./sql/schema/"
  engine: postgresql
  gen:
    go:
      out: internal/db
      package: db
      emit_json_tags: true
      emit_prepared_queries: false
      emit_interface: true
version: '2'
sql:
- queries: sql/db
  schema: src/migrations
  engine: postgresql
  gen:
    go:
      package: db
      out: src/lib/db
      emit_prepared_queries: true
      emit_json_tags: true
      overrides:
      - go_type: github.com/foo/bar.Baz
        column: foo
    kotlin:
      package: com.example.db
      out: src/main/kotlin
      overrides:
      - gen_type: okio.ByteString
        db_type: ByteArray
gen:
  go:
    overrides:
    - go_type: github.com/foo/bar.Baz
      db_type: integer
    rename:
      sid: SID
  kotlin:
    overrides:
    - gen_type: com.example.Foo
      db_type: jsonb

@kyleconroy
Copy link
Collaborator Author

kyleconroy commented Jan 30, 2020

Does this new syntax assumes that the default driver is the PostgreSQL one, or this still doesn't contemplate MySQL?

Great point. We should take this change to make engine required. I've updated the configuration file to have an engine field in each queries item.

@mightyguava
Copy link
Contributor

I still don't think generating the same schema into multiple languages is something sqlc needs to support, and doing so adds an additional level of nesting in the config. Probably not much complexity though. LGTM!

While we are extending the config, I just remembered another want: #314. Can we account for that as well, assuming you think the request is useful?

@kyleconroy
Copy link
Collaborator Author

I still don't think generating the same schema into multiple languages is something sqlc needs to support, and doing so adds an additional level of nesting in the config.

Fair enough. As we add more languages, we'll be able to see if anyone is using the multi-language support. If not, we'll take that into account for version 3

@eullerpereira94
Copy link
Contributor

Instead of specifying the language on config file and make it even more cluttered, can we add a lang flag in the command tool that defaults to Go?

@koodimetsa
Copy link

It's generally preferred nowadays to use camelCase in JSON

Version 1 used snake_case, so I'm inclined to keep snake_case going forward for compatibility reasons. I don't think moving to a different case format is worth a breaking change.

Is there a possibility to get camel casing with a flag? This would be great feature in few cases. For example a web-app which uses multiple APIs and some of them return camel case. Would be great if everything would use the same casing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants