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

DatumReader as a generic datum reader #64

Closed

Conversation

yadavsaroj
Copy link

As per discussion here with @rdblue, I have moved binary data related knowledge to the binary decoder and have made datum_reader a generic datum reader.

@yadavsaroj yadavsaroj changed the title Yadavsaroj/generic datum reader DatumReader as a generic datum reader Dec 12, 2015
@@ -318,7 +350,7 @@ def read_fixed(writers_schema, readers_schema, decoder)
end

def read_enum(writers_schema, readers_schema, decoder)
index_of_symbol = decoder.read_int
index_of_symbol = decoder.read_enum
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this pass the writer's schema so that the JSON decoder can find the appropriate offset?

Copy link
Author

Choose a reason for hiding this comment

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

Not really. As per specs An enum is encoded by a int, representing the zero-based position of the symbol in the schema..

@rdblue
Copy link
Contributor

rdblue commented Dec 14, 2015

Thanks @yadavsaroj, this looks like it is close.

My suggestion about reading enums is that you might need to pass the schema to the encoder. That makes me wonder if it wouldn't be easier to add this to your other pull request, #56, so that you're sure that the API is going to work for both. What do you think?

- s/items_count/block_count/
@yadavsaroj
Copy link
Author

@rdblue just took care of your comments. Please let me know how it looks.

About merging PR to my other branch, I would like to merge small independent PRs at a time to the master if you don't mind. If I need additional changes when working on other branch, I can always do it in that PR.

@asfgit asfgit closed this Feb 4, 2016
iemejia pushed a commit that referenced this pull request Jun 11, 2021
Commit 35d2108 converted many source files to use Rust 2018
features (e.g., `use crate`).  These changes make avro-rs impossible
to build with older versions of the compiler.

    ~/src/avro-rs$ rustc --version
    rustc 1.29.2 (17a9dc751 2018-10-05)

    ~/src/avro-rs$ cargo check
        Checking avro-rs v0.6.4 (file:///home/vincent.foley/src/avro-rs)
    error[E0658]: `crate` in paths is experimental (see issue #45477)
      --> src/codec.rs:12:5
       |
    12 | use crate::types::{ToAvro, Value};
       |     ^^^^^

    error[E0658]: `crate` in paths is experimental (see issue #45477)
      --> src/codec.rs:13:5
       |
    13 | use crate::util::DecodeError;
       |     ^^^^^

    [Many more errors]

    error: aborting due to 33 previous errors

    For more information about this error, try `rustc --explain E0658`.
    error: Could not compile `avro-rs`.

    To learn more, run the command again with --verbose.

This commit adds an explicit declaration in Cargo.toml that Rust 2018
is needed to build avro-rs.  The build still fails with older versions
of the compiler, but at the reason is clearer.

    ~/src/avro-rs$ rustc --version
    rustc 1.29.2 (17a9dc751 2018-10-05)

    ~/src/avro-rs$ cargo check
    error: failed to parse manifest at `/home/vincent.foley/src/avro-rs/Cargo.toml`

    Caused by:
      editions are unstable

    Caused by:
      feature `edition` is required

    this Cargo does not support nightly features, but if you
    switch to nightly channel you can add
    `cargo-features = ["edition"]` to enable this feature
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants