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

WITH SYSTEM VERSIONING (mariaDB) causes print-schema to fail #3570

Open
3 tasks done
tgross35 opened this issue Mar 28, 2023 · 4 comments
Open
3 tasks done

WITH SYSTEM VERSIONING (mariaDB) causes print-schema to fail #3570

tgross35 opened this issue Mar 28, 2023 · 4 comments

Comments

@tgross35
Copy link
Contributor

Setup

Versions

  • Rust: rustc 1.70.0-nightly (1db9c061d 2023-03-21)
  • Diesel: diesel 2.0.1
  • Database: mariadb
  • Operating System macos

Feature Flags

  • diesel: "mysql_backend", "serde_json", "chrono", "uuid"

Problem Description

Enabling tables with system versioning (WITH SYSTEM VERSIONING) means there is no output in the schema.rs file.

What are you trying to accomplish?

Generate the schema using diesel print-schema

What is the expected output?

Some invocation of the diesel::table! macro in schema.rs

What is the actual output?

No output (just the comment)

Are you seeing any additional errors?

None

Steps to reproduce

System versioned tables are supported in MariaDB (also in IBM DB2, and SQL server, current WIP with PG) but using them seems like Diesel can't reflect the tables. For example:

CREATE TABLE foo (
    x serial,
    primary key (x)
) WITH SYSTEM VERSIONING;

Creates no output. Dropping WITH SYSTEM VERSIONING generates create output

diesel::table! {
    foo (x) {
        x -> Unsigned<Bigint>,
    }
}

I haven't tried it, but I would have to imagine that application versioning doesn't work either

Context

MySQL doesn't yet support system versioning, so the issue probably stems from some incompatibility there. There are a few small but growing incompatibilities, so it might make sense to have a mariadb connection string option (SQLAlchemy went this route for some features where autodetection didn't work well)

The clause WITH SYSTEM VERSIONING adds two columns hidden by default: ROW_START and ROW_END. It also creates a quasi-column ("period" type) called SYSTEM_TIME that can be selected with a special syntax (select * from my_table for SYSTEM_TIME all). These column & period names can also be specified. When updating a row, a system versioned table just duplicates the row and changes the end date, to keep a history of all changes without triggers (and hides the historical rows by default).

There is also application versioning, which is a bit different but uses similar new syntax & temporal constructs.

Ideally, I think it would be good for Diesel to provide a way to work with system versioned tables (e.g. .filter(foo).system_time(all)) since they're slowly popping up in more database flavors. But for now, I'd just be happy if the tables reflect to Rust properly, ignoring any system versioning constructs (though adding the ROW_START and ROW_END columns to reflection would be quite awesome)

Links:

If diesel would want to add some more specific SV/AV support, I'd be happy to help architect

Checklist

  • This issue can be reproduced on Rust's stable channel. (Your issue will be
    closed if this is not the case)
  • This issue can be reproduced without requiring a third party crate
@tgross35 tgross35 added the bug label Mar 28, 2023
@weiznich
Copy link
Member

Thanks for filling this issue.
Diesel uses the information schema to query information about the database. It might be that WITH SYSTEM VERSIONING tables just do not appear there. At least that's what I would check first.

That written: As it's currently, we do not officially support mariadb, but only mysql. So all features/bugs that are mariadb specific are unfortunate but are not really considered to be bugs with the mysql backend. At some point in the past we decided that we would accept an additional backend for mariadb (see #1882). That would allow to address these low level differences in a much more targeted way. That backend is waiting of someone to sit down and write it. At least I personally do currently not have the capacity for that, but contributions there are welcome.

@tgross35
Copy link
Contributor Author

Thanks for the response, seems like there's a couple reasons for a dedicated mariadb backend. What would be the bare minimum PR to get support moving in the right direction? It seems like duplicating diesel/diesel/src/mysql and updating the connection would be needed, but I'm not sure how many of the other sections of the repo would need to be touched just to get something going, or what a prioritized todo list would look like.

(feel free to reply at #1882)

@tgross35
Copy link
Contributor Author

Here's a dump from information_schema using specifically the mysql command line: foo has SV enabled, baz has SV and AV (bitemporal), and bar has nothing

+---------------+--------------+------------+------------------+--------+---------+------------+------------+----------------+-------------+-----------------+--------------+-----------+----------------+---------------------+-------------+------------+--------------------+----------+----------------+---------------+------------------+-----------+
| TABLE_CATALOG | TABLE_SCHEMA | TABLE_NAME | TABLE_TYPE       | ENGINE | VERSION | ROW_FORMAT | TABLE_ROWS | AVG_ROW_LENGTH | DATA_LENGTH | MAX_DATA_LENGTH | INDEX_LENGTH | DATA_FREE | AUTO_INCREMENT | CREATE_TIME         | UPDATE_TIME | CHECK_TIME | TABLE_COLLATION    | CHECKSUM | CREATE_OPTIONS | TABLE_COMMENT | MAX_INDEX_LENGTH | TEMPORARY |
+---------------+--------------+------------+------------------+--------+---------+------------+------------+----------------+-------------+-----------------+--------------+-----------+----------------+---------------------+-------------+------------+--------------------+----------+----------------+---------------+------------------+-----------+
| def           | db           | baz        | SYSTEM VERSIONED | InnoDB |      11 | Dynamic    |          0 |              0 |       16384 |               0 |            0 |         0 |           NULL | 2023-03-28 17:03:47 | NULL        | NULL       | utf8mb4_general_ci |     NULL |                |               |                0 | N         |
| def           | db           | foo        | SYSTEM VERSIONED | InnoDB |      10 | Dynamic    |          0 |              0 |       16384 |               0 |        16384 |         0 |              1 | 2023-03-28 09:31:24 | NULL        | NULL       | utf8mb4_general_ci |     NULL |                |               |                0 | N         |
| def           | db           | bar        | BASE TABLE       | InnoDB |      10 | Dynamic    |          0 |              0 |       16384 |               0 |        16384 |         0 |              1 | 2023-03-28 17:01:58 | NULL        | NULL       | utf8mb4_general_ci |     NULL |                |               |                0 | N         |
+---------------+--------------+------------+------------------+--------+---------+------------+------------+----------------+-------------+-----------------+--------------+-----------+----------------+---------------------+-------------+------------+--------------------+----------+----------------+---------------+------------------+-----------+

It looks like the only difference is in BASE TABLE vs. SYSTEM VERSIONED for TABLE_TYPE, looks like it is checked here

.filter(table_type.like("BASE TABLE"))
. I'll make a PR which just allows for either table type, since they interchangeable to the frontend, for all intents and purposes

@weiznich
Copy link
Member

Thanks for investigating and putting up a PR. That's at least a good starting point.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants