Conversation
|
@newville I would appreciate your review and comments. |
| files are therefore regular SQLite files which use a specific database schema, | ||
| defined by this document. | ||
|
|
||
| The file extension `.zarrdb` is **recommended** for Zarr SQLiteStore files. |
There was a problem hiding this comment.
is this an extension that sqlite tools will recognize? something like .zarr.db might work better if not
There was a problem hiding this comment.
I don't know that there is really a standard file extension.
There was a problem hiding this comment.
My preference is for a single extension, it's easier to work with using splitext and similar functions. There is no standard extensions for sqlite, .db and .sqlite are somewhat common, but it is also common to see app-specific extensions for their application.
|
@auxym Thanks very much. I think this looks really good. But I will also readily admit that I know very little about the ZEP process or expectations. I might suggest stating that this schema is "version 1.0" and that other schema (perhaps "table per group") are conceivable in the future, but would need an update to the spec and updates to supporting libraries. For metadata fields, I agree that "created-by" is more general and suitable for the general spec. I might say that some value like "created_datetime" could be listed as "optional metadata", as original timestamps can be lost. If we are bike-shedding the schema, I might have a slight preference for "key" and "value" or "val" over "k" and "v", but that is minor.
'.db' is used by many tools. It is used for many sqlite3 databases, but, '.db3', '.sqlite' and '.sqlite3' are also common. I don't know of any sqlite tools that enforce ".db". I think '.zarrdb' is a fine default, but agree most strongly with the "recommended" here. |
|
|
||
| | Key | Required | Description of value | | ||
| |-----|----------|----------------------| | ||
| | `sqlitestore_version` | Yes | Version of the SQLiteStore file format in `MAJOR.MINOR` format. As of the present document, this should be the string "1.0". | |
There was a problem hiding this comment.
This could instead be stored as pragma user_version.
There was a problem hiding this comment.
The Spec here conveniently specifies finite rules for the tables (and exactly two of them) to used by Zarr. It does not forbid other tables in the same database. That might be very convenient for application-specific files that want to store some data in "chunked, compressed numerical data" but also store other data in multi-column tables, maybe even really with relations (or just prefer to store a list of strings or json structures using "plain" datatypes instead of as Zarr arrays). Zarr itself would not see those other tables, but a downstream app could.
I think that the sqlite3 pragmas for user_version or application_data are then probably best left to the users and applications, not to the Zarr spec.
There was a problem hiding this comment.
That is an interesting consideration --- do you anticipate that to be a likely use case?
An advantage of user_version and application_id is that they can be used for file-format auto-detection quite easily, because they are at fixed offsets from the start of the file. E.g. the standard file tool could easily be updated to indicate that it is a zarr sqlite store. Potentially they could be recommended for format auto-detection but not required.
If the intent is to allow both zarr and non-zarr data in the same database you might also consider using a clearer name than sqlitestore for the metadata table, e.g. zarr_sqlite_metadata.
There was a problem hiding this comment.
Excellent idea for the application ID, I was not aware of this feature and I will look into it.
For the user_version, I'm a bit conflicted. On one hand, it's a fixed offset value and makes it easy to determine the schema version without having to call into sqlite. On the other hand, I liked having "everything in one place" (the metadata table).
@newville concerning the possibility of adding extra tables -- I think it would probably be best to just leave that unspecified. If people want to do it, it won't break readers, but they should know they are doing something non-standard. Future versions of the spec may define feature flags that could signal, for example, other tables with whatever in them.
| files are therefore regular SQLite files which use a specific database schema, | ||
| defined by this document. | ||
|
|
||
| The file extension `.zarrdb` is **recommended** for Zarr SQLiteStore files. |
There was a problem hiding this comment.
It would be nice to get an assigned application_id for this format. https://sqlite.org/pragma.html#pragma_application_id
| - Full ACID guarantees provided by SQLite. | ||
| - High availability of SQLite implementations across programming languages | ||
| and environments. | ||
|
|
There was a problem hiding this comment.
A key disadvantage to be aware of is that the standard sqlite c implementation performs all i/o sequentially and reads a page at a time Therefore the read throughput will be quite low for high latency storage, like s3, gcs, etc. although you can try to mitigate that with readahead heuristics or use an aggregative alternative implementation.
There was a problem hiding this comment.
sorry, but "a key disadvantage" compared to what?
There was a problem hiding this comment.
I am really not nowledgeable enough on sqlite to comment on this. I will however update the spec to have a more neutral tone. This information (advantages/disadvantages) would better go in the README, I think.
For s3/blob storage, zarr already supports native storage backend that will likely be more performant. The goal of SQLiteStore, for me, was never really about better performance (compared to zip, s3).
|
Another important caveat with sqlite format is that individual large rows, including blobs are inherently stored as a linked list of overflow pages. Therefore reading a large value may be slow on high latency storage, because each page will have to be read sequentially, and byte range reads of individual values will also not be particularly efficient. |
|
The largest binary blob size is 2 GiB in sqlite, but we will likely want to keep the blobs relatively small due to the overflow page mechanism. I also suggest that the specification include some recommendations about setting the page size and some consideration about aligning the Zarr chunk size and the sqlite page size when compression is not being used. I think we would want the Zarr chunk size to be a small multiple of the page size. https://www.sqlite.org/pragma.html#pragma_page_size It seems to me that we would want to set the page size to be the largest value in most cases. With 64 KiB pages, a 1 MB chunk would only consist of at most 16 overflow pages. Another consequence of this paging structure is that we should recommend against using sharding within a sqlite file. |
This may be tricky because some portion of the value will be stored inline and the rest in the overflow page.
A downside of a large page size is more wasted space, because an overflow page can only hold a single row. Therefore, the space consumed by the overflowing portion of the value is rounded up to a multiple of the page size.
Yes, indeed. |
|
This specification is mainly written as a specifiation to store a Zarr array into a single sqlite file. That seems appropriate for arrays of a certain scale. For arrays of a large scale, we may want to partition (shard) the array among several sqlite files. At minimum, I think we should add some scope statements to the specification that as written as single file this intended for arrays of several gigabytes at most. I also see some need for cloud-optimization should the sqlite files be stored in remote cloud store like S3. It may make sense to retrieve and cache information from the dbstat table. For example, we may want to cache the byte offsets of pages and figure out how to implement a virtual file system layer to aggregate small partial reads into larger partial reads. This could be used so that reading a single chunk can be done via a single S3/HTTP request which would mitigate the overflow paging issues discussed above. That said, I would like to see the multiple file case addressed at some point, but this may be a distinct specification for a codec analogous to the current core sharding_indexed codec. Perhaps here it could just be stated as potential alternative that is out of scope for this specification. |
|
As I see it, this sqlite store is best suited for moderate size zarr hierarchies (total size less than ~10GB) intended for read/write use on a local filesystem or low-latency network filesystem. For cloud storage, or large datasets that you would want to split over large files, I don't think this is a great fit --- something like icechunk or OCDBT (supported by tensorstore and neuroglancer) would almost certainly be better. |
|
It is great that SQLite pragmas can be adjusted to improve efficiencies, but I would be reluctant to have these hard-coded in the Spec. It might be OK for the spec to state which pragmas should be settable from the API when creating a SQLiteStore, and maybe recommend defaults. Having guidance in user-level documentation might be good enough. I don't see such options for Zip or LocalFile storage. There is general guidance on "how to think about setting chunk sizes" (though maybe I am confusing it with HDF5), but that seems mostly independent of storage. I agree that "50 GB or smaller" is a fine assumption for SQLiteStore (and probably ZipStore too). It's awesome that Zarr can handle much larger datasets and supports access over cloud storage with fsspec, etc, but that might not be the main use case for SQLiteStore. As a data producer, and supporting analysis tools for such datasets, I'm assuming that data-producing and data-analyzing tools would create SQLiteStore files on behalf of users (basically, in place of HDF5, also in the ~50GB or smaller range). Such applications are probably the best place to make assumptions about array sizes, and the number of distinct groups and arrays that would be stored, and so are probably the right place to set such options. Making those options clear and giving guidance on setting values would be great. |
Agreed, I will add this mention to the spec. I wonder, if a future version adds an "alternative schema", would this be defined by a flag? Or maybe we should add a new record "schema_type" or similar to the metadata table?
Yeah I don't see a big downside to adding an optional datetime record to metadata, I'll add it.
I agree but since it is mostly inconsequential, I thought keeping some compabibility with the zarr-v2 implementation (which used k/v) was preferable. |
I agree, I will make this change. Regarding various performance optimizations being discussed (page size and whatnot): since they do not break file-level compatibility between implementations, I think they should be left unspecified and implementation specific. I am open to future revisions of the spec including some performance recommendations. |
|
@auxym Thanks, yes I agree that pragmas probably do not change the spec. That is, unless one wants to spell out which pragmas must be exposed at db creation. But I think it is okay to decide that the spec leaves pragmas up to downstream apps and users. I think it might be enough to say that a valid SQLiteStore v1 MUST have a table named 'sqlitestore_metadata' table, which MUST have a value for key= |
|
|
I played with an sqlite store before and thought about splitting it across 2 tables, one mapping zarr chunk keys to content hashes and then the chunks themselves in a table keyed on the content hash. That way you get deduplication, if that's of interest, and your key table rows are of more predictable size which might make listing and existence checks smoother. |
|
@mkitti Thanks for the reference to the OME-Zarr spec. |
|
@clbarnes I think that could be very interesting. Do you have any suggestions for how the Spec (and API) could be altered for that? Performance info and tuning would be great, but probably needs realistic example datasets. Perhaps asking for too much (or sorry for the ignorance if it already exists), but is there an obvious set of examples or test data sets? That seems like it would be very helpful for comparing FileStore, ZipStore, and SQLiteStore, and choices for the schema of SQLiteStore, different filesystems for FileStore, or the available options for ZipStore. That's slightly off-topic for this Spec, but it seems like it would make much of the discussion here more concrete. |
That's an interesting idea. I see two drawbacks:
Is de-duplication an appreciable gain? How common are duplicate chunks in zarr datasets? My intuition says "probably very rare", except maybe for trivial data (full array of 0's) which compresses very well anyways. Concerning the gain on row size, unfortunately I'm not knowledgeable enough of sqlite internals to judge. If someone if interested in this, maybe they could create a few benchmarks? I'm not against going in this direction if it demonstrates measurable performance gains. Otherwise, I would propose keeping this idea as an alternative format/schema which could be added to a future version of the spec. |
|
I agree that duplicated chunks would be rare and that hashing could be expensive, but the sqlite store would probably be best suited for small chunks (=cheaper hashes and more likely duplication). The implementation complexity is pretty minor, but still likely not worth bothering with. |
|
I would suggest against adding something like hashing/deduplication to this store. It is the kind of thing that could be achieved though a storage adapter (akin to icechunk) or even better a storage transformer. |
|
probably simplicity is a virtue for the initial spec. unless there are use cases that are doomed without hashing / deduplication / other elaborations, maybe this spec should just commit to the simplest thing for starters |
|
Hi all, thank you for your comments and suggestions. I have made a commit with many modifications to the spec.
I think we are converging on something I can merge soon. I'd once again appreciate your review, comments and recommendations. |
|
I don't have anything technical to add but I just want to say that this direction looks great, thanks to everyone for working on this |
|
Oh! Also I forgot, I would need guidance from zarr devs (@d-v-b @LDeakin @jbms) on the Canonical URI section, which I added but left empty. Sqlite itself natively supports I did notice that the ZipStore proposal includes the URI prefix |
I don't know that a canonical URL is required, but according to my URL pipeline proposal zip should be: file:///path/to/archive.zip|zip:path/within/archive and similarly for this proposal it could be: file:///path/to/database.zarrdb|sqlite:path/within/sqlitestore |
|
Regarding the URI, I was referring to this section of the zarr core spec:
|
|
Thanks -- I think URI is a good discussion point.
I don't see that added backslash at ZipStore proposal. Aside from a knee-jerk "yuck" at mixing forward and backslashes, I suggest using actual URI conventions Following the suggestion from @auxym, something like would probably be a reasonable option. Using would be a fine option too. With that approach, it is imaginable that in an idealized future, |
|
Hm, would that mean that timestamps with non-UTC time zones are forbidden for an optional field? If a timestamp conforms to RFC-3339, isn't the 'T' between date and time not required? Since this is in the SPEC, are implementations expected to enforce this strict formatting? Should an invalid "created_time" be rejected silently or does that invalidate the entire store? Seems like a pretty strict requirement for an optional field. |
According to RFC-3339, yes. The spec here is a constraint on top of RFC-3339, which is why it's explicit that it is an RFC-3339 datetime with T and Z.
The choices are
As was brought up elsewhere, we should go with the robustness principle:
i.e. strictly specify what writers should write (i.e. |
|
Also, since the field is optional, readers are allowed to treat the timestamp as missing if the format does not meet the spec. |
- Rename `sqlitestore_metadata` table to `zarr_sqlitestore_metadata` - rename `create_time` record to `modified_at` - Specify that readers should not rely on the accuracy of the `modified_at` timestamp - Suggest use of COLLATE BINARY to ensure correct (case sensitive) key comparison (this is the default)
Clarified that writers are not required to update the modified_at field.
|
Thanks again all, I just made a few edits reflecting recent comments. Note in particular the renaming of the I still plan to add a non-normative section containing suggested SQL queries for store operations. |
This is great! We should probably test interop between both libraries at some point. FWIW I just pushed to branch dev (https://github.com/auxym/zarr-sqlite-python/tree/dev) and it currently implements the latest revision of this spec. |
Sorry to be dense, but does "robust" imply "more strict" or "more tolerant"? I might not read "robust" as "therefore, timestamps missing a 'T', or having a timezone, or not in UTC are invalid". I agree that readers should not parse date times: they should rely on standard libraries. Or, perhaps for this spec, on SQLite. For Python, I would defer to SQLite's Disallowing formatting here that both SQLite and Python's standard library support is hard for me to understand. |
|
Robust here means "clearly specify exactly what should be written" and "allow readers to parse other (sensible) things if they want". In this case, give an exact strftime format string to be written, but make sure it's within specs like ISO-8601 and RFC-3339 so that people can plug in more general parsers on the reading side just in case they can divine meaning from badly-behaved writers. You make a good point that we're in sqlite-land and so should use something which plays nice with sqlite datetime formats. I don't feel strongly about the T being present, other than it being the preferred state for RFC-3339. I would prefer the Z be present to remove time zone ambiguity, but it looks like that kills interop with sqlite's built-in date/time functions, so instead I would say
Support for different numbers of decimal places is pretty spotty across different languages so we could scrap subsecond precision entirely, which would further point towards this field being for human information, not machine integrity checks. Particular formats being convenient in one particular language is, IMO, not a good argument for choosing that particular format in the spec (except for sqlite, the "native" language for this file format), especially when controlling the format is usually very easy (it certainly is in python). |
That's an interesting idea, I wasn't previously familiar with sqlite's date and time functions. Note however that our current timestamp format spec is also possible to produce in plain sqlite sql using update zarr_sqlitestore_metadata set v=strftime('%Y-%m-%dT%H:%M:%fZ', 'now', 'utc') where k='modified_at';And also that the format is a natively understood "time value" in SQLite ("format 7" as listed here: https://sqlite.org/lang_datefunc.html in section 2). So we could keep the current rfc-3339 spec and add an informative note saying it can be produced by this SQL. |
|
Adding a default UTC timestamp for this implementation would be great. But if the Spec says "MUST", it MUST mean it. I think that is not so easy. Perhaps the Spec should specify the exact MUST is a lot different from SHOULD. |
I agree.
I didn't think this was the case because none of the time formats given on that page show time zones, but I've experimented and you're right. Despite not being documented (!?), sqlite does understand ISO-8601 offsets, including
As is always the case for sqlite. I am not particularly invested in the timestamp fields' existence - IMO if it's important to the zarr consumer, they should store it in their zarr metadata (not our problem), and if it's important to the owner of the database, they can use their filesystem's metadata. Certainly if datetime format bikeshedding takes up any more of our time, I'd prefer to just drop those fields. But if we are to have timestamp fields, they should:
In that order. Leaving open the possibility for any timezone other than UTC breaks both 2 and 4. If sqlite is capable of internally normalizing any RFC-3339 timestamp so client code doesn't have to, I'm happy to go with the current wording plus MUST have timezone information ( |
It is documented, but maybe a bit easy to gloss over on the docs page:
|
|
My mistake, thanks for pointing it out. |
I note here should.
Yes, they should.
Sorry, I do not agree. Saying "timestamps should use RFC-3339 formatting" would satisfy your 5 criteria.
That doesn't agree with your conclusion that any timezone other than UTC breaks "trivially interoperable" or "minimize implementation effort". Again, MUST is very different from "should". If you say MUST, then it is easy to interpret a non-compliant value as invalidating the Store. It is also easy to interpret this to mean that an implementation is obligated to validate these values for every access. A downstream user can do update zarr_sqlitestore_metadata set v='Wednesday afternoon' where k='modified_at'; What is a Zarr SQLite store expected to do in that situation? MUST the Store immediately become invalid? What happens if one implementation does not do that invalidation? Again, it is great to have any implementation use UTC or values with timezone information - highly recommended. This is most definitely not bike-shedding. It is about ensuring that a Spec can be taken seriously and implemented uniformly. |
In my opinion, it should probably treat it as missing. It is optional anyways and unnecessary for parsing data out of the store. The current python implementation never reads the timestamp at all, in fact. On the UTC issue: In light of my recent reading of the SQLite docs on its date and time function, I think we should keep "obligatory UTC with explicit Z", for this reason: SQLite treats all timestamps without timezone data as UTC. Explicitly requiring UTC prevents a case where missing tz information is ambiguous between UTC or some unspecified time zone. Anyways, as was already demonstrated, the RFC-3339 UTC timestamp format can be both written and parsed internally in SQLite. |
I suppose I meant "should" in the colloquial sense rather than SHOULD in the RFC-2119 sense.
I'm mainly erring on the side of "time zones are hard". Temporal spent 9 years in development and some browsers still haven't implemented it. Yes, there are decent implementations in many languages, but "it's all UTC and there's a Z here to prove it" is much simpler. Allowing flexibility in the datetime format raises a questions like "should you try to roundtrip the exact same format when you update it", as well as the marginal awkwardness of having to parse two datetimes to compare them rather than being able to do a straight string comparison. Anyone can write software which is non-compliant with any spec you care to name. In my opinion, the spec should require (MUST) |
|
My objection is to the word MUST.
I find that very confusing. If the Spec says MUST, then how does the reader have a choice? If one tries to open a Zarr-sqlite store with It seems perfectly believable to me that someone looking to write an interface would look at that situation and conclude that they are indeed obligated to reject the Store. The Spec makes very few claims about requirements for data values, but it does very clearly state that this value of Having MUST in a Spec should not be taken lightly. That's all from me. @auxym Thanks for doing this. I look forward to the day that I can use Zarr with SQLite. |
|
OK, I can add some wording to the tune of "Readers should treat a modified_at with an invalid format as if the modified_at record is absent", if that helps clarify things. |
|
@auxym The Spec says it uses IETF RFC 2119. There are many choices other than MUST. Timestamps in UTC are most definitely NOT a MUST for this Spec. |
|
What do you propose exactly as an alternative? |
|
I have added a non-normative appendix that includes SQL statements that can be used to achieve all store operations defined by the core spec (exception for partial writes, I don't it can be achieved in plain SQLite SQL without using the blob API), as well as metadata-related operations (including working with timestamps). |
| Thus, for non-empty prefixes, a prefix search may be implemented as: | ||
|
|
||
| ```sql | ||
| WHERE k > :prefix |
There was a problem hiding this comment.
Is there a performance advantage of this form over WHERE k LIKE CONCAT(:prefix, '%')?
There was a problem hiding this comment.
LIKE in sqlite is case-insensitive and there doesn't seem to be an easy way to set it to be case-sensitive (other than overwriting the compare function with a custom stored function). Also we'd need to escape the % character in keys if present (not a huge deal admittedly).
My implementation initially used GLOB, which is case sensitive, but has the problem that it doesn't support escaping the special glob characters at all, from what I could find (*[]?).
| INSERT INTO zarr(k, v) | ||
| VALUES (:key, :value) | ||
| ON CONFLICT(k) | ||
| DO UPDATE SET v = excluded.v; |
There was a problem hiding this comment.
Is there an advantage of this form over INSERT OR REPLACE INTO ...?
There was a problem hiding this comment.
Not a huge advantage, but this preserves the rowid. INSERT OR REPLACE INTO effectively deletes the row and inserts a new row.
"SHOULD" or "RECOMMENDED" are very different from MUST. The table names are "MUST" because the Store cannot function without these. You can have MUST for version information (but SHOULD, with a listed default value, would be OK too) as that could impact what the library does with the tables. There is no proposed internal use of the creation or modified dates: they are information meant for communication with the user, and maybe for tools organizing the data. Yes, good timestamps are to be encouraged. The data is not invalid if it is missing. |
|
Sorry but once again could you clarify? Are you proposing
or
Or something else entirely? |
|
@auxym Trying to be as polite as I possibly can, and beyond the risk and actually at repeating myself more than once: I believe that MUST should not occur in the descriptions of the timestamps. I recommend that they both say Again, from https://datatracker.ietf.org/doc/html/rfc2119#section-1, the meaning of MUST is an absolute requirement of the specification. of the entire Spec for Zarr-Sqlite, not the spec for one component. Again, suggesting very strongly that a non-compliant value here invalidates the full Zarr-Sqlite store. Yes, really, that is what that means. Zarr-sqlite MUST have tables named "zarr" and "zarr_sqlite_metadata". An email address MUST have an "@" sign. This has nothing to do with how SQLite or any other software formats dates. It very much has to do with the meaning of the word MUST. There are so few published ZEPs and so little guidance from the Zarr developers, and so much passing the buck between "Zarr-python" and "Zarr core" that it really is hard to know how seriously to take this process. |
|
@newville I agree with you that an invalid timestamp should not invalidate all the data otherwise in the store. That was not the intention of the current wording. Let me now expose my concern (and I believe @clbarnes, from my understanding of his previous comments, though I do not want to speak for him).
I fear that if we leave it at this, then we are effectively leaving the timestamp format undefined, which is a complicated situation for readers/parsers. I had a re-read of RFC-2119 and noted these sections (emphasis is mine):
In which particular circumstances should a writer ignore the recommended (SHOULD) timestamp format, and write another format?
If we are to understand "interoperation" here as "readers being able to parse a timestamp written by any reader, as long as both are spec-conforming", then I believe that the timestamp format should be imposed (MUST) for writers. Otherwise, a writer could write a timestamp in lunar months (ridiculous example to make a point, sorry), and still be considered technically spec-conforming, but how can we expect any reader implementation to handle this? Please interpret my questions here as genuinely trying to understand all concerns and find a reasonable solution, and not as any sort of attack. Personally, I would prefer wording similar to the following:
I believe this addresses your main concern: it clarifies that readers should not invalidate actual data if the timestamp is invalid, it is just treated as absent, which is a valid state for the store, as this piece of metadata is optional. It also leaves readers allowed to be more permissive than required when interpreting the timestamp, if they wish, for example by using Do you see any problems I would've missed if we choose to include this last section in the spec? |
|
Here's some relevant "prior art" on similar topics: From the PNG spec (https://www.w3.org/TR/png-3/) (emphasis mine again)
From the HTTP spec (https://www.rfc-editor.org/rfc/rfc9110.html)
To my knowledge, most browsers and HTTP clients do not handle one invalid HTTP header by rejecting the entire request, even though the header format definitions are full of MUSTs. |
Here is a draft spec for the SQLiteStore. It is quite simple, but I figure we should at least start with something. I think we should agree on a somewhat stable spec before this package is published to PyPI and files start profilerating.
Metadata table.
Similar to what was proposed in #2, this spec adds a second table which is used exclusively to write data which is necessary for the interpretation of the store format.
I decided against storing the following fields:
The metadata table is not yet implemented in the python lib.
I am open to any comments, modifications, additions. I will merge this PR and publish a 1.0 version of this package to PyPI once we reach a sort of consensus on the spec.