Skip to content

Conversation

chloebarre
Copy link
Collaborator

No description provided.

Copy link
Collaborator

@RiwsPy RiwsPy left a comment

Choose a reason for hiding this comment

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

Merci !
J'ai mis quelques remarques pas bloquantes.

).fetchall()
existing_cols = {col[0] for col in columns_sql}

missing_cols = set(schema.keys()) - existing_cols
Copy link
Collaborator

Choose a reason for hiding this comment

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

Tu savais que dict.keys() possède une compatibilité partielle avec les set ?
missing_cols = schema.keys() - existing_cols serait suffisant ici.

return

# Mapping Polars -> SQL
type_mapping = {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Il y a un bout de code qui a l'air de faire l'inverse dans Historisateur.convert_types_for_sql.
Ce dictionnaire pourrait être utile ailleurs dans le code, je le sortirai de la méthode pour en créer une constante.

Comment on lines 96 to 101
for col in missing_cols:
pl_type = schema[col]
sql_type = type_mapping.get(pl_type, "TEXT")
sql = text(f'ALTER TABLE "{table_name}" ADD COLUMN "{col}" {sql_type};')
conn.execute(sql)
conn.commit()
Copy link
Collaborator

Choose a reason for hiding this comment

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

C'est bon sur le principe, idéalement il faudrait réduire les appels à la db en concaténant les requêtes pour n'en faire qu'une seule.

Peut-être quelque chose comme ça ?

        add_columns = [… for col in missing_cols]
        sql = text(f'ALTER TABLE "{table_name}" {", ".join(add_columns)};)
        conn.execute(sql)
        conn.commit()

@RiwsPy RiwsPy changed the title pb de nouvelle colonne dans la database [BACK] pb de nouvelle colonne dans la database Jul 16, 2025
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