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

Add partitioned tables type in postgres schema generation #405

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

dimonmontana
Copy link

@dimonmontana dimonmontana commented Mar 6, 2024

Hello, this is my attempt to implement supporting partitioned tables in Postgres schema generation. Currently, such tables are skipped from generation.

List of changes:
models/table.xo.go - add one more c.relkind type for table definition
types/types.go - Add Partition struct to define table partition properties
cmd/schema.go - Update LoadTables with adding table partition information
gen.sh - Update PostgresTables query
templates/createdb/xo.xo.sql.tpl - Add supporting postgres inherits tables and indexes

@kenshaw
Copy link
Member

kenshaw commented Mar 6, 2024

I see. This query is actually generated from gen.sh -- I'd be happy to incorporate this change if you also added it to the gen.sh script. I would also like if you could add an example in the a_bit_of_everything example that has a partitioned table.

@dimonmontana
Copy link
Author

@kenshaw Thanks for details! I hope everything is included now

@kenshaw
Copy link
Member

kenshaw commented Mar 6, 2024

Hi, could you please rebase this as a single commit? I'll review in the morning.

@dimonmontana
Copy link
Author

@kenshaw kindly remind you about these changes

@kenshaw
Copy link
Member

kenshaw commented Mar 12, 2024

@dimonmontana this breaks the templates for the other databases :( I'm looking to see if I'm able to fix it now.

@dimonmontana
Copy link
Author

@kenshaw thanks for the quick response! Could you please point me how to reproduce such breaks? I tried to regenerate a_bit_of_everything with SQL, MySQL, and postgres and everything was generated as expected without any issues.

@kenshaw
Copy link
Member

kenshaw commented Mar 13, 2024

@dimonmontana The issue was on Microsoft SQL Server and Oracle. Not sure if you are using the usql code base here, but there's scripts in the usql contrib directory that will launch/run containers for the primary test databases:

b$ ./podman-run.sh test
-------------------------------------------
NAME:       mysql 
IMAGE:      docker.io/library/mariadb (update: 0)
PUBLISH:    3306:3306
ENV:        MYSQL_ROOT_PASSWORD=P4ssw0rd
VOLUME:     
NETWORK:    
PRIVILEGED: 
CMD:        
+ podman stop mysql
mysql
+ podman run --detach --rm --name=mysql --publish=3306:3306 --env=MYSQL_ROOT_PASSWORD=P4ssw0rd docker.io/library/mariadb
4de33df9f25a8acc74cc876919d3bd1e22c9d717486983dd7abe00fa72bf30c6
-------------------------------------------
NAME:       postgres 
IMAGE:      docker.io/usql/postgres (update: 0)
PUBLISH:    5432:5432
ENV:        POSTGRES_PASSWORD=P4ssw0rd
VOLUME:     
NETWORK:    
PRIVILEGED: 
CMD:        
+ podman stop postgres
postgres
+ podman run --detach --rm --name=postgres --publish=5432:5432 --env=POSTGRES_PASSWORD=P4ssw0rd docker.io/usql/postgres
f94dad491ba99016867f6cdc139a7957460ff13d0d901fca37e3940551a3cdb8
-------------------------------------------
NAME:       sqlserver 
IMAGE:      mcr.microsoft.com/mssql/server:2022-latest (update: 0)
PUBLISH:    1433:1433
ENV:        ACCEPT_EULA=Y MSSQL_PID=Express SA_PASSWORD=Adm1nP@ssw0rd
VOLUME:     
NETWORK:    
PRIVILEGED: 
CMD:        
+ podman stop sqlserver
sqlserver
+ podman run --detach --rm --name=sqlserver --publish=1433:1433 --env=ACCEPT_EULA=Y --env=MSSQL_PID=Express --env=SA_PASSWORD=Adm1nP@ssw0rd mcr.microsoft.com/mssql/server:2022-latest
0c80eed640fefefef125c2aa7e6678fad28e1ebe7420143f949e7559e2b8f769
-------------------------------------------
NAME:       oracle 
IMAGE:      container-registry.oracle.com/database/free (update: 0)
PUBLISH:    1521:1521
ENV:        ORACLE_PDB=db1 ORACLE_PWD=P4ssw0rd
VOLUME:     oracle-data:/opt/oracle/oradata
NETWORK:    
PRIVILEGED: 
CMD:        
+ podman stop oracle
oracle
+ podman run --detach --rm --name=oracle --publish=1521:1521 --env=ORACLE_PDB=db1 --env=ORACLE_PWD=P4ssw0rd --volume=oracle-data:/opt/oracle/oradata container-registry.oracle.com/database/free
21fdd9eed9d0142e9cd0bd7ce657c0535faa91357366eed8b3ef7068aebc13a7
-------------------------------------------
NAME:       clickhouse 
IMAGE:      docker.io/clickhouse/clickhouse-server (update: 0)
PUBLISH:    9000:9000
ENV:        CLICKHOUSE_DEFAULT_ACCESS_MANAGEMENT=1 CLICKHOUSE_USER=clickhouse CLICKHOUSE_PASSWORD=P4ssw0rd
VOLUME:     
NETWORK:    
PRIVILEGED: 
CMD:        
+ podman stop clickhouse
clickhouse
+ podman run --detach --rm --name=clickhouse --publish=9000:9000 --env=CLICKHOUSE_DEFAULT_ACCESS_MANAGEMENT=1 --env=CLICKHOUSE_USER=clickhouse --env=CLICKHOUSE_PASSWORD=P4ssw0rd docker.io/clickhouse/clickhouse-server
6f45e1d023e0ce3d3b23d0aec1981ca9d4938d5e8fd90b47f2a2693dcfe9a82c
-------------------------------------------
NAME:       cassandra 
IMAGE:      docker.io/usql/cassandra (update: 0)
PUBLISH:    9042:9042
ENV:        
VOLUME:     
NETWORK:    
PRIVILEGED: 
CMD:        
+ podman stop cassandra
cassandra
+ podman run --detach --rm --name=cassandra --publish=9042:9042 docker.io/usql/cassandra
ef8c4702866a1d25cdc1731bb9fbc5272b39b1de8775a77d2a4d6bb8cc7aedd1

And of course the usql/contrib/usqlpass file is the configuration for all the test databases, as used by the xo gen scripts:

$ cp usql/contrib/usqlpass $HOME/.usqlpass && chmod 0600 $HOME/.usqlpass
$ cat $HOME/.usqlpass
# sample ~/.usqlpass file
# 
# format is:
# protocol:host:port:dbname:user:pass
postgres:*:*:*:postgres:P4ssw0rd

cql:*:*:*:cassandra:cassandra
clickhouse:*:*:*:clickhouse:P4ssw0rd
godror:*:*:*:system:P4ssw0rd
ignite:*:*:*:ignite:ignite
mymysql:*:*:*:root:P4ssw0rd
mysql:*:*:*:root:P4ssw0rd
oracle:*:*:*:system:P4ssw0rd
pgx:*:*:*:postgres:P4ssw0rd
sqlserver:*:*:*:sa:Adm1nP@ssw0rd
vertica:*:*:*:vertica:P4ssw0rd
flightsql:*:*:*:flight_username:P4ssw0rd

@dimonmontana
Copy link
Author

Hey @kenshaw,
Thanks for the information about usql it was really helpful with reproducing issues. I found that the issue happens when the generator tries to fill the view comments (https://github.com/xo/xo/pull/405/files#diff-ae46cf3a492bbffcdd8edfcbd966c04f7abc2fe1eb2c427c2dcaed3b9dbe45f5R225).

I will be glad to hear about the result of the generation from your side as well. Thanks in advance!

@dimonmontana
Copy link
Author

Hi @kenshaw, did you have a chance to look into this?

@grachevko
Copy link

Any news?

@grachevko
Copy link

I use this branch in prod, work as expected.

@dimonmontana
Copy link
Author

Hey @grachevko,
Thanks for double-checking this! I hope soon @kenshaw or someone else will review the changes and merge them!

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