Skip to content

Types: Improve type mappings#24

Merged
amotl merged 1 commit into
mainfrom
amo/more-typemapping
Jun 4, 2026
Merged

Types: Improve type mappings#24
amotl merged 1 commit into
mainfrom
amo/more-typemapping

Conversation

@amotl
Copy link
Copy Markdown
Contributor

@amotl amotl commented Jan 16, 2024

About

  • Consequently use upper-case type definitions from sqlalchemy.types
  • Add timestamp without time zone types (scalar and array)
  • On SQLAlchemy 2, map real and double{_precision} types to the
    newly introduced sqltypes.{DOUBLE,DOUBLE_PRECISION} types

All of this is intended to improve reverse type lookups / reflections.

References

@amotl amotl force-pushed the amo/more-typemapping branch from 9324bf1 to 120f8ea Compare January 16, 2024 01:02
@amotl amotl changed the title Types: Improve type mapping for scalars and arrays Types: Improve type mappings Jan 16, 2024
Comment thread src/sqlalchemy_cratedb/dialect.py Fixed
@amotl amotl force-pushed the amo/more-typemapping branch 3 times, most recently from daa7029 to fea2527 Compare January 16, 2024 01:36
@amotl amotl force-pushed the amo/more-typemapping branch 2 times, most recently from 4732e07 to a63ce6e Compare June 25, 2024 14:54
@amotl amotl force-pushed the amo/more-typemapping branch 2 times, most recently from c3efc82 to e6837c0 Compare March 30, 2025 02:24
@amotl amotl force-pushed the amo/more-typemapping branch from e6837c0 to 1a0e14b Compare April 11, 2025 19:32
@amotl
Copy link
Copy Markdown
Contributor Author

amotl commented Apr 11, 2025

Dear @CaselIT,

may we humbly ask you to give this patch a quick review? We started to overhaul the type mapping of the CrateDB dialect the other day, and would like to conclude this patch.

Other than filling some gaps, we found that mapping database types to the uppercase type symbol variants provided by SQLAlchemy and back (i.e. using sqltypes.VARCHAR rather than sqltypes.String), offers better (reverse) type mapping behaviour with downstream frameworks like pandas, Meltano, or previous versions of Ibis and friends. Do you also think it is the right choice to do it this way?

With kind regards,
Andreas.

@CaselIT
Copy link
Copy Markdown

CaselIT commented Apr 11, 2025

for reflected tables? it's fine, sqlalchemy types have as_generic so if users need to adapt the types to other db they can simply use that in the reflect event.

@amotl
Copy link
Copy Markdown
Contributor Author

amotl commented Apr 11, 2025

Thank you!

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Jun 2, 2026

Important

Review skipped

Auto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: fa2c934e-e773-41c7-b781-aef269e81553

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch amo/more-typemapping

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@amotl amotl requested a review from bgunebakan June 2, 2026 20:33
@amotl amotl marked this pull request as ready for review June 2, 2026 20:33
@amotl amotl requested a review from kneth June 3, 2026 10:02
@bgunebakan bgunebakan removed the request for review from kneth June 3, 2026 15:47
Copy link
Copy Markdown

@bgunebakan bgunebakan left a comment

Choose a reason for hiding this comment

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

Thank you, There are 2 small things. Could you check?

Comment thread src/sqlalchemy_cratedb/dialect.py Outdated
Comment thread src/sqlalchemy_cratedb/dialect.py Outdated
@amotl amotl requested a review from bgunebakan June 4, 2026 10:54
Copy link
Copy Markdown
Member

@kneth kneth left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Copy Markdown

@bgunebakan bgunebakan left a comment

Choose a reason for hiding this comment

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

Thanks!

@amotl amotl force-pushed the amo/more-typemapping branch from 5280304 to b1a1b94 Compare June 4, 2026 21:17
- Consequently use upper-case type definitions from `sqlalchemy.types`
- Add `timestamp without time zone` types (scalar and array)
- On SQLAlchemy 2, map `real` and `double{_precision}` types to the
  newly introduced `sqltypes.{DOUBLE,DOUBLE_PRECISION}` types

All of this is intended to improve reverse type lookups / reflections.
@amotl amotl force-pushed the amo/more-typemapping branch from b1a1b94 to 345f75d Compare June 4, 2026 21:17
@amotl amotl merged commit 1205083 into main Jun 4, 2026
38 checks passed
@amotl amotl deleted the amo/more-typemapping branch June 4, 2026 21:23
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.

5 participants