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

Make PostgreSQL ready for Gramps 6.0 #638

Draft
wants to merge 1 commit into
base: maintenance/gramps60
Choose a base branch
from

Conversation

DavidMStraub
Copy link
Member

I have not tested this yet. Will leave as draft until it's ready to merge.

"""
self.__cursor.execute(
"SELECT COUNT(*) FROM information_schema.columns "
"WHERE table_name = %s AND column_name = %s",
Copy link
Member

Choose a reason for hiding this comment

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

Does postgresql have this convention:

Suggested change
"WHERE table_name = %s AND column_name = %s",
"WHERE table_name = ? AND column_name = ?",

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, that's what psycopg2 uses - it's like that in the rest of the addon as well.

@DavidMStraub
Copy link
Member Author

By the way - since TEXT is hard-coded in DBAPI, we cannot use native JSON types, correct?

@dsblank
Copy link
Member

dsblank commented Feb 5, 2025

By the way - since TEXT is hard-coded in DBAPI, we cannot use native JSON types, correct?

One of the many aspects we learn as we port to different engines 😄

In order for Postgresql to use a different type, we'll have to have an abstraction for that in the base class. Like:

class DBAPI():
    json_data_field_type = "TEXT

and use that when we create the fields. There may be a couple of other abstractions if we have a generic SQL query method. I'll make a PR for the json_data_field_type in the base DBAPI class.

@dsblank
Copy link
Member

dsblank commented Feb 5, 2025

I may have misunderstood your question: I think Postgresql can use whatever type you want, as long as it acts like a string in Python.

@DavidMStraub
Copy link
Member Author

Actually, after reading up a bit, I realize I was making a wrong assumption - I thought the JSON type was more performant or offered more possibilities to query it, but it turns out it doesn't do much compared to TEXT, only enforcing JSON syntax, which we don't really need. So, nothing to do right now ☺️

It will get more interesting/complicated if/when we start to query into the JSON structure itself, where each engine has their own syntax; like I had to do for the Sifts search library used in Gramps Web, e.g.

https://github.com/DavidMStraub/sifts/blob/main/src/sifts/core.py#L428
vs
https://github.com/DavidMStraub/sifts/blob/main/src/sifts/core.py#L573

@dsblank
Copy link
Member

dsblank commented Feb 6, 2025

It will get more interesting/complicated if/when we start to query into the JSON structure itself

That is on the schedule for 6.1 (soon!). We should coordinate now to make sure that whatever we do, that it works across at least postgresql and sqlite.

Can you test this query in postgresql on new database structure:

SELECT json_extract(json_data, '$.primary_name.surname_list[0].surname') 
FROM person 
WHERE (json_extract(json_data, '$.gender') = 0) 
ORDER BY json_extract(json_data, '$.handle') DESC, json_extract(json_data, '$.gender');

That will be a good start. We'll have to check operators, but there is a test suite that I'll port to whatever approach we take.

@DavidMStraub
Copy link
Member Author

Sorry, but I have no capacity to check these things before the 6.0 release. Can we please not start new breaking database backend changes before 6.0 is fully released?

@DavidMStraub
Copy link
Member Author

drop_column still needs to be added

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.

2 participants