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

016 Database Adapters and Indices #21

Merged
merged 9 commits into from
May 4, 2022
Merged

016 Database Adapters and Indices #21

merged 9 commits into from
May 4, 2022

Conversation

eldadfux
Copy link
Member

@eldadfux eldadfux commented Mar 15, 2021

@kodumbeats
Copy link
Contributor

I'm not fully convinced that a MongoDB adapter makes sense - we'd be abstracting a document store with another document store. Though, I can understand how/why devs end up on the free-forever Atlas tier and why they'd like the compatibility 🤔

@lohanidamodar
Copy link
Member

Wow in pretty good shape already. MongoDB support with disclosed Limitation should be good. It's been a one of the popular choice as a database for quite some time now.

@kodumbeats
Copy link
Contributor

kodumbeats commented Mar 16, 2021

For enterprise support, we should look into supporting ACID transactions:

Another database driver that could be interesting is sqlite. Largest advantage is that sqlite databases are single files, which allows for a ton of portability (basically serverless).

Copy link
Contributor

@TorstenDittmann TorstenDittmann left a comment

Choose a reason for hiding this comment

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

Already in a good state 👍🏻

* Array
* Object
* Relationships
* Reference (collection / document)
Copy link
Contributor

Choose a reason for hiding this comment

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

This is important and mighty!

Especially since we don't have to nest documents and documents can be referenced in many places.

* Null

##### Complex Types
* Array
Copy link
Contributor

Choose a reason for hiding this comment

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

I assume Array can be used with any Type, including array?

Copy link
Member Author

Choose a reason for hiding this comment

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

Not sure, I originally thought to limit it to contain only one primitive type - also to have easier representation in GraphQL.

016-database-adapters.md Show resolved Hide resolved
@christyjacob4
Copy link
Member

@eldadfux If we are doing a refactoring, it might be a good idea to include a layer of redis cache as part of Utopia\Database itself. This way, we (and other devs) could use Utopia\Database as a standalone library for their APIs

@eldadfux
Copy link
Member Author

@eldadfux If we are doing a refactoring, it might be a good idea to include a layer of redis cache as part of Utopia\Database itself. This way, we (and other devs) could use Utopia\Database as a standalone library for their APIs

Agree.

Copy link
Member

@christyjacob4 christyjacob4 left a comment

Choose a reason for hiding this comment

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

Things look good @eldadfux . Would like to follow up when a POC is ready which will bring to light more challenges that we haven't thought of yet 😅

@kodumbeats
Copy link
Contributor

Some thoughts about custom indexes:

How will users know which attributes to index? The standard approach is to use the EXPLAIN statement to understand what's happening under the hood, but our abstraction doesn't allow users to write their own queries. Could leverage a product like EverSQL and design interfaces for selecting proper attributes to index.

Hosting questions:

Where/how do we set storage limits?

A list of potential limits that other providers use:

  • Memory/CPU
  • Storage capacity
  • Row/document limits
  • Frequency/capacity for backups
  • Query timeouts
  • Region availability / sharding
  • Rollbacks

References:

@TorstenDittmann
Copy link
Contributor

Addendum to the meeting and as a reminder. Topic: Unique Types.

Perhaps these should not necessarily be associated with indexes. Also, should this be done at database or adapter level?

@eldadfux
Copy link
Member Author

Addendum to the meeting and as a reminder. Topic: Unique Types.

Perhaps these should not necessarily be associated with indexes. Also, should this be done at database or adapter level?

I think we can leverage the specific adapter implementation for better confidence and performance. Should work fine with all adapters.

@kodumbeats
Copy link
Contributor

kodumbeats commented Mar 22, 2021

Unanswered question

sqlite3 doesn't use schemas, but instead gets the schema-name from the attached database (which is a file, in these cases). What should Adapter->create() look like with files instead of schemas?

@eldadfux
Copy link
Member Author

Unanswered question

sqlite3 doesn't use schemas, but instead gets the schema-name from the attached database (which is a file, in these cases). What should Adapter->create() look like with files instead of schemas?

A separate file might be a good equivalent

@kodumbeats
Copy link
Contributor

Will collections have their own read/write permissions after this refactor?

* create
* delete

**Collections** (Tables for SQL)
Copy link
Contributor

Choose a reason for hiding this comment

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

Collections, attributes, and indexes seem to get a $name, and documents have an $id. Will all of these resources get an $id in the background? Or will we just use $name and $id as written?


#### Orders

Support multi-column orders, multiple order type (ASC, DESC). We should leverage native casting for all types and avoid any manual casting of the data.
Copy link
Member

Choose a reason for hiding this comment

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

Can we support Random order?
This is one of the biggest still existing issue with Firebase as well.

@kodumbeats
Copy link
Contributor

From recent discussion
We've decided to move forward with a dot notation + chaining approach to client filters and discussed URL encoding. We propose the following delimiters:

URI
?filter[]=movie.director.name/equal/'Michael Bay'&filter[]=movie.director.gender/equal/'male'

Array
filter = ["movie.director.name/equal/'Michael Bay'", "movie.director.gender/equal/'male'"]

Unresolved questions

  • Should we create a class for ordering, so we can pass the object as a param? Something like:
    db.listDocuments($collection, $filter, $order)

@eldadfux
Copy link
Member Author

eldadfux commented Apr 8, 2021

From recent discussion
We've decided to move forward with a dot notation + chaining approach to client filters and discussed URL encoding. We propose the following delimiters:

URI
?filter[]=movie.director.name/equal/'Michael Bay'&filter[]=movie.director.gender/equal/'male'

Array
filter = ["movie.director.name/equal/'Michael Bay'", "movie.director.gender/equal/'male'"]

Unresolved questions

  • Should we create a class for ordering, so we can pass the object as a param? Something like:
    db.listDocuments($collection, $filter, $order)

Why did we move away from the functions like syntax?

If it is for performance enhancement I think we should take the small penalty for a more intuitive function like syntax. This will also be easier to explain in the docs, and will allow more options in the future like geo location and search "functions".

@TorstenDittmann
Copy link
Contributor

From recent discussion
We've decided to move forward with a dot notation + chaining approach to client filters and discussed URL encoding. We propose the following delimiters:

URI
?filter[]=movie.director.name/equal/'Michael Bay'&filter[]=movie.director.gender/equal/'male'

Array
filter = ["movie.director.name/equal/'Michael Bay'", "movie.director.gender/equal/'male'"]

Unresolved questions

  • Should we create a class for ordering, so we can pass the object as a param? Something like:
    db.listDocuments($collection, $filter, $order)

Why did we move away from the functions like syntax?

If it is for performance enhancement I think we should take the small penalty for a more intuitive function like syntax. This will also be easier to explain in the docs, and will allow more options in the future like geo location and search "functions".

If you are good with the penalty - either approach is fine with me.

@kodumbeats
Copy link
Contributor

Unanswered question

sqlite3 doesn't use schemas, but instead gets the schema-name from the attached database (which is a file, in these cases). What should Adapter->create() look like with files instead of schemas?

A separate file might be a good equivalent

I had trouble creating a SQLite adapter due to its differences in PDO. A SQLITE PDO connection is made to a single file database, not just to the RDBMS:

$pdo = new PDO("sqlite:".$filePath);

So, after constructing a SQLite adapter, the create() abstract method's behavior can't be replicated.

Do you have any thoughts/suggestions?

@eldadfux
Copy link
Member Author

Unanswered question

sqlite3 doesn't use schemas, but instead gets the schema-name from the attached database (which is a file, in these cases). What should Adapter->create() look like with files instead of schemas?

A separate file might be a good equivalent

I had trouble creating a SQLite adapter due to its differences in PDO. A SQLITE PDO connection is made to a single file database, not just to the RDBMS:

$pdo = new PDO("sqlite:".$filePath);

So, after constructing a SQLite adapter, the create() abstract method's behavior can't be replicated.

Do you have any thoughts/suggestions?

We should strive to reach a situation where all the differences between different adapters are handled in the constructor method. You can pass the path, store it in the instance and use it later.

@lohanidamodar
Copy link
Member

@eldadfux @kodumbeats @TorstenDittmann Anyone thinking about Database Transactions during refactoring? Can we support those?

@lohanidamodar
Copy link
Member

@eldadfux @kodumbeats Support for server side timestamps. Like document creation, update timestamps. may be option for auto filled server side timestamps. 🤔

@lohanidamodar
Copy link
Member

@eldadfux @kodumbeats Pointer
Can we add support for updating Array elements:
https://firebase.google.com/docs/firestore/manage-data/add-data#update_elements_in_an_array

Can we add support for updating nested fields (if we support JSON type) https://firebase.google.com/docs/firestore/manage-data/add-data#update_fields_in_nested_objects

@JaviBonilla
Copy link

JaviBonilla commented Jun 3, 2021

@eldadfux @kodumbeats @TorstenDittmann Anyone thinking about Database Transactions during refactoring? Can we support those?

Transactions (set of read / write atomic operations) would be definetively great to prevent inconsistency issues in the database

Comment on lines +70 to +74
The list of supported adapters will include:
* Postgres
* MySQL
* MariaDB
* MongoDB¹

Choose a reason for hiding this comment

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

When it comes time to write documentation for these changes, I think it would be really nice to have a section outlining the differences between the supported databases.

Although I know by design they will all function the same, there might be subtle differences that developers might want to know.

Developers using Appwrite will most likely care about:

  • Disk size of database
  • Portability, (ease of backup/restore)
  • Speed
  • Resource cost (CPU, memory, etc)

Choose a reason for hiding this comment

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

Just a comment for your consideration. Are you considering aggregation functions? Such as count, sum, max, min, etc. of a particular rule in the collection, this could be quite useful.

https://en.m.wikipedia.org/wiki/Aggregate_function

Copy link
Member Author

Choose a reason for hiding this comment

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

Probably not in step one, but I can definitely see the value here as we build the future roadmap of this API

@fightforlife
Copy link

Will the relationships part of the query operations be filled with things like AND and OR?
https://github.com/appwrite/rfc/blob/910e7e015c7516c643272f3981ee349dc36c5d11/016-database-adapters.md#joins--relationships
Relevant Issue:
appwrite/appwrite#1174

@kodumbeats
Copy link
Contributor

Hi @fightforlife 👋🏻

Our idea for relationships is to have a way to associate one collection with another, and we're still working on the design/technical implementation of this feature.

Our current filters work as a list of AND statements, and our refactor adds support for OR statements. This is what the syntax will look like:
https://gist.github.com/kodumbeats/83df7d970096db9e7ba998669e554dee

@alexweininger
Copy link

Can a collection have more than one index?

@eldadfux
Copy link
Member Author

Can a collection have more than one index?

Yes. We are currently following the under the hood db limits for max number of indexes.

For MariaDB: ~64
For MySQL: ~64
For MongoDB: ~64

@eldadfux eldadfux marked this pull request as ready for review May 4, 2022 22:50
@eldadfux eldadfux merged commit 7b2e1f1 into main May 4, 2022
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.

8 participants