-
Notifications
You must be signed in to change notification settings - Fork 80
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
Add parquet_ans_processor #455
Conversation
} | ||
} | ||
|
||
// Parse V1 ANS write set changes |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We need to also parse V2 ANS write set changes
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why do we have to parse V2 ans wsc?
rust/processor/src/processors/parquet_processors/parquet_ans_processor.rs
Show resolved
Hide resolved
67ec5f1
to
6e3a15e
Compare
pub txn_version: i64, | ||
pub write_set_change_index: i64, | ||
pub registered_address: String, | ||
pub token_standard: String, | ||
pub domain: Option<String>, | ||
pub subdomain: Option<String>, | ||
pub token_name: Option<String>, | ||
pub is_deleted: bool, | ||
#[allocative(skip)] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ying-w let me know if it's missing any fields
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
missing expiration_timestamp
and subdomain_expiration_policy
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@rtso do you know how to parse out the expiration timestamps?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Primary names don't have expiration timestamp, only ans lookups do. This PR looks like it's missing moving ans_lookup_v2 to Parquet @yuunlimm
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
all_ans_lookups_v2, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I also realized that this processor is still writing to the v1 ANS tables
all_current_ans_lookups,
all_ans_lookups,
all_current_ans_primary_names,
all_ans_primary_names,
We can include these in the deprecated tables and turn it off
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think ans_lookup_v2 was a part of parquet migration.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ying-w do we need lookups in BQ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
just realized this was for ans_primary_name_v2
not ans_lookup_v2
Yes we use both ans_lookup_v2
and current_ans_primary_name_v2
and current_ans_lookup_v2
if it's just removing ans_primary_name_v2
i think that is ok w/out moving to parquet but if going to remove current that would like the non-current to be moved to parquet
6e3a15e
to
10efc4b
Compare
#[async_trait] | ||
impl ProcessorTrait for ParquetAnsProcessor { | ||
fn name(&self) -> &'static str { | ||
ProcessorName::AnsProcessor.into() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
do we want a processor name that can differentiate parquet and regular ones? I see this method is implemented in different ways: 1) from processor name 2) hard coding
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
good catch! will fix this.
76a0e6b
to
96228f8
Compare
Description
Test Plan