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

Feat(perf) change cast from string to int #468

Closed
wants to merge 1 commit into from

Conversation

Pierre-Narcisi
Copy link
Contributor

@Pierre-Narcisi Pierre-Narcisi commented Aug 31, 2023

Amélioration des performances en passant un une comparaison entre string en une en une comparaison en Int qui est beaucoup plus rapide.

#424

Lignes importées Int String 
100 0.015 0.14 
1000 0.14 0.28 
10000 1.39  2.90
100000 18.31 23.26 

Copy link
Contributor

@bouttier bouttier left a comment

Choose a reason for hiding this comment

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

C’est très bien le try_cast_int ! Mais c’est dommage de l’utiliser que pour cd_nom / cd_hab. Il me semble plus opportun de :

  • activer le check de type pour cd_nom / cd_hab
  • modifier le check de référentiel pour utiliser la colonne de synthèse (complété par le check de type) et ainsi ne plus avoir de cast à faire
  • modifier le check de type pour utiliser try_cast_int plutôt que le check dataframe



def downgrade():
pass
Copy link
Contributor

Choose a reason for hiding this comment

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

drop function

@camillemonchicourt
Copy link
Member

Remplacé par #482

@bouttier
Copy link
Contributor

bouttier commented Oct 3, 2023

À noter que cette PR introduit la fonction SQL try_cast_int qui n’est pas présent dans #482 qui se limite à la suppression du cast. Le travail initié dans cette PR peut donc encore servir pour convertir les checks de types dataframes vers SQL.

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.

3 participants