Skip to content

Disable logical type cast of fastavro #34603

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.

Sign up for GitHub

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

Open
wants to merge 13 commits into
base: master
Choose a base branch
from
Open

Disable logical type cast of fastavro #34603

Changes from all commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
f1d9e93
Log Warning if process function return None
DKER2 Sep 24, 2024
3cdb629
Fix get function without inner
DKER2 Sep 25, 2024
9170808
check the first def_line also
DKER2 Sep 25, 2024
9370ed2
rename variable
DKER2 Sep 25, 2024
6219664
add strip function
DKER2 Sep 25, 2024
5a3c1a6
reformat function
DKER2 Sep 25, 2024
3bfc43d
refactor code
DKER2 Sep 25, 2024
278d186
fix bug in get function body
DKER2 Sep 29, 2024
f07d2cd
Merge branch 'master' into master
DKER2 Sep 29, 2024
e819366
retrigger test
DKER2 Sep 29, 2024
25d5431
retrigger test
DKER2 Sep 29, 2024
5161a11
fix: disable cast to logical type
DKER2 Apr 10, 2025
7c0dd2f
refactor: revert redundant change
DKER2 Apr 10, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions sdks/python/apache_beam/io/avroio.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -163,6 +163,8 @@ def __init__(
super().__init__()
self._source = _FastAvroSource(
file_pattern, min_bundle_size, validate=validate)
# Disable fastavro's automatic logical type conversion
fastavro.read.LOGICAL_READERS.clear()
Copy link
Collaborator

Choose a reason for hiding this comment

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

This will remove all avro LOGICAL_READERS in both the case where someone outputs as_row, or not meaning whatever the atomic value of the logical type is will be passed and all other info will be lost..

This might break for people who depend on the current LOGICAL_READERS behavior when as_row=False. If we decide to pass timsetamp-millis as integers, a more targeted fix would be to remove the LOGICAL_READER for 'long-timestamp-millis' only if as_row=True.

Also worth considering whether we want to treat the "timestamp-millis" as a Timestamp like mentioned in #31656 (comment).

@Abacn do you have an opinion whether the "timestamp-millis" type should be treated as an integer or a "Timestamp" type?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah agree, a generic suggestion is to avoid breaking change to users.

if as_rows:
path = FileSystems.match([file_pattern], [1])[0].metadata_list[0].path
with FileSystems.open(path) as fin:
Expand Down
Loading