You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@thrift.apache.org by "Nathan Marz (JIRA)" <ji...@apache.org> on 2008/12/04 20:46:44 UTC
[jira] Created: (THRIFT-223) Validate method should check that enum
types are assigned valid values
Validate method should check that enum types are assigned valid values
----------------------------------------------------------------------
Key: THRIFT-223
URL: https://issues.apache.org/jira/browse/THRIFT-223
Project: Thrift
Issue Type: Improvement
Components: Compiler (Java)
Reporter: Nathan Marz
Priority: Minor
The validate method generated currently checks that required fields are set. It would be nice if it were to enforce more parts of the schema. One example of this are the values assigned to enum types. For example, if I have this enum:
enum MyEnum {
FOO = 1;
BAR = 3;
BAZ = 4;
BIZ = 5;
}
and this struct:
struct MyStruct {
MyEnum e;
}
The validate method would ensure that MyStruct#e is either 1, 3, 4, or 5.
The naive way of implementing this would be to generate a conditional statement for every value, aka
"e==1 || e==3 || e==4 || e==5"
A better implementation would generate something like:
"e==1 || (e>=3 && e<=5)"
Since the common case seems to be having large ranges of contiguous values, this is the difference between having N conditionals execute versus 2.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (THRIFT-223) Validate method should check that
enum types are assigned valid values
Posted by "Bryan Duxbury (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/THRIFT-223?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12660910#action_12660910 ]
Bryan Duxbury commented on THRIFT-223:
--------------------------------------
I think that we can't really help the enums in containers problem. It's solvable, but I think we should open a separate issue for that, as it's related to a lot of other things that need to be fixed with containers.
I think you're right about the __isset branching. Maybe what we should do is add a method to the generator that generates the isset check for a given field? Whatever we do, there will be some issues with the way it is now.
> Validate method should check that enum types are assigned valid values
> ----------------------------------------------------------------------
>
> Key: THRIFT-223
> URL: https://issues.apache.org/jira/browse/THRIFT-223
> Project: Thrift
> Issue Type: Improvement
> Components: Compiler (Java)
> Reporter: Nathan Marz
> Assignee: Piotr Kozikowski
> Priority: Minor
> Attachments: thrift-223-v2.patch, thrift-223-v3.patch, thrift-223-v4.patch, thrift-223-v5.patch, thrift-223.patch
>
>
> The validate method generated currently checks that required fields are set. It would be nice if it were to enforce more parts of the schema. One example of this are the values assigned to enum types. For example, if I have this enum:
> enum MyEnum {
> FOO = 1;
> BAR = 3;
> BAZ = 4;
> BIZ = 5;
> }
> and this struct:
> struct MyStruct {
> MyEnum e;
> }
> The validate method would ensure that MyStruct#e is either 1, 3, 4, or 5.
> The naive way of implementing this would be to generate a conditional statement for every value, aka
> "e==1 || e==3 || e==4 || e==5"
> A better implementation would generate something like:
> "e==1 || (e>=3 && e<=5)"
> Since the common case seems to be having large ranges of contiguous values, this is the difference between having N conditionals execute versus 2.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (THRIFT-223) Validate method should check that enum
types are assigned valid values
Posted by "Piotr Kozikowski (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/THRIFT-223?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Piotr Kozikowski updated THRIFT-223:
------------------------------------
Attachment: thrift-223-v5.patch
Renamed constantsSet to validValues to make it consistent with latest implementation of Thrift-224.
> Validate method should check that enum types are assigned valid values
> ----------------------------------------------------------------------
>
> Key: THRIFT-223
> URL: https://issues.apache.org/jira/browse/THRIFT-223
> Project: Thrift
> Issue Type: Improvement
> Components: Compiler (Java)
> Reporter: Nathan Marz
> Assignee: Piotr Kozikowski
> Priority: Minor
> Attachments: thrift-223-v2.patch, thrift-223-v3.patch, thrift-223-v4.patch, thrift-223-v5.patch, thrift-223.patch
>
>
> The validate method generated currently checks that required fields are set. It would be nice if it were to enforce more parts of the schema. One example of this are the values assigned to enum types. For example, if I have this enum:
> enum MyEnum {
> FOO = 1;
> BAR = 3;
> BAZ = 4;
> BIZ = 5;
> }
> and this struct:
> struct MyStruct {
> MyEnum e;
> }
> The validate method would ensure that MyStruct#e is either 1, 3, 4, or 5.
> The naive way of implementing this would be to generate a conditional statement for every value, aka
> "e==1 || e==3 || e==4 || e==5"
> A better implementation would generate something like:
> "e==1 || (e>=3 && e<=5)"
> Since the common case seems to be having large ranges of contiguous values, this is the difference between having N conditionals execute versus 2.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (THRIFT-223) Validate method should check that enum
types are assigned valid values
Posted by "Piotr Kozikowski (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/THRIFT-223?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Piotr Kozikowski updated THRIFT-223:
------------------------------------
Attachment: thrift-223-v6.patch
Ok, I didn't notice that __isset was updated in the reader but not in the writer. The way I see it, it's not possible to know if a primitive type has been set in non-beans style, so the "// alas, we cannot check" branch is necessary. Here is an updated patch that does the following:
-field-values-getter on generated structs was removed
-beans style generated code remains the same
-for non-beans style, the validate method only checks for non-primitive required types, and the rest are checked only in the reader
-validate checks for valid enum values only for fields that are set in __isset, so in non-beans style it won't do the check unless the object was read (deserialized).
Also, I'm not sure I understand what you mean by "isset check" in "add a method to the generator that generates the isset check for a given field"
> Validate method should check that enum types are assigned valid values
> ----------------------------------------------------------------------
>
> Key: THRIFT-223
> URL: https://issues.apache.org/jira/browse/THRIFT-223
> Project: Thrift
> Issue Type: Improvement
> Components: Compiler (Java)
> Reporter: Nathan Marz
> Assignee: Piotr Kozikowski
> Priority: Minor
> Attachments: thrift-223-v2.patch, thrift-223-v3.patch, thrift-223-v4.patch, thrift-223-v5.patch, thrift-223-v6.patch, thrift-223.patch
>
>
> The validate method generated currently checks that required fields are set. It would be nice if it were to enforce more parts of the schema. One example of this are the values assigned to enum types. For example, if I have this enum:
> enum MyEnum {
> FOO = 1;
> BAR = 3;
> BAZ = 4;
> BIZ = 5;
> }
> and this struct:
> struct MyStruct {
> MyEnum e;
> }
> The validate method would ensure that MyStruct#e is either 1, 3, 4, or 5.
> The naive way of implementing this would be to generate a conditional statement for every value, aka
> "e==1 || e==3 || e==4 || e==5"
> A better implementation would generate something like:
> "e==1 || (e>=3 && e<=5)"
> Since the common case seems to be having large ranges of contiguous values, this is the difference between having N conditionals execute versus 2.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (THRIFT-223) Validate method should check that
enum types are assigned valid values
Posted by "David Reiss (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/THRIFT-223?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12661312#action_12661312 ]
David Reiss commented on THRIFT-223:
------------------------------------
bq. I think that we can't really help the enums in containers problem. It's solvable, but I think we should open a separate issue for that, as it's related to a lot of other things that need to be fixed with containers.
Okay
bq. Maybe what we should do is add a method to the generator that generates the isset check for a given field?
Sounds good.
> Validate method should check that enum types are assigned valid values
> ----------------------------------------------------------------------
>
> Key: THRIFT-223
> URL: https://issues.apache.org/jira/browse/THRIFT-223
> Project: Thrift
> Issue Type: Improvement
> Components: Compiler (Java)
> Reporter: Nathan Marz
> Assignee: Piotr Kozikowski
> Priority: Minor
> Attachments: thrift-223-v2.patch, thrift-223-v3.patch, thrift-223-v4.patch, thrift-223-v5.patch, thrift-223.patch
>
>
> The validate method generated currently checks that required fields are set. It would be nice if it were to enforce more parts of the schema. One example of this are the values assigned to enum types. For example, if I have this enum:
> enum MyEnum {
> FOO = 1;
> BAR = 3;
> BAZ = 4;
> BIZ = 5;
> }
> and this struct:
> struct MyStruct {
> MyEnum e;
> }
> The validate method would ensure that MyStruct#e is either 1, 3, 4, or 5.
> The naive way of implementing this would be to generate a conditional statement for every value, aka
> "e==1 || e==3 || e==4 || e==5"
> A better implementation would generate something like:
> "e==1 || (e>=3 && e<=5)"
> Since the common case seems to be having large ranges of contiguous values, this is the difference between having N conditionals execute versus 2.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (THRIFT-223) Validate method should check that enum
types are assigned valid values
Posted by "Piotr Kozikowski (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/THRIFT-223?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Piotr Kozikowski updated THRIFT-223:
------------------------------------
Attachment: thrift-223-v2.patch
Here is a patch that follows the new approach. Now every field of type enum will have a corresponding user accessible fieldValuesSet set containing all valid values for that enum. The validation performs logically the same function as before.
For enums with a very large number of consecutive values the first approach might still be better, though.
> Validate method should check that enum types are assigned valid values
> ----------------------------------------------------------------------
>
> Key: THRIFT-223
> URL: https://issues.apache.org/jira/browse/THRIFT-223
> Project: Thrift
> Issue Type: Improvement
> Components: Compiler (Java)
> Reporter: Nathan Marz
> Assignee: Piotr Kozikowski
> Priority: Minor
> Attachments: thrift-223-v2.patch, thrift-223.patch
>
>
> The validate method generated currently checks that required fields are set. It would be nice if it were to enforce more parts of the schema. One example of this are the values assigned to enum types. For example, if I have this enum:
> enum MyEnum {
> FOO = 1;
> BAR = 3;
> BAZ = 4;
> BIZ = 5;
> }
> and this struct:
> struct MyStruct {
> MyEnum e;
> }
> The validate method would ensure that MyStruct#e is either 1, 3, 4, or 5.
> The naive way of implementing this would be to generate a conditional statement for every value, aka
> "e==1 || e==3 || e==4 || e==5"
> A better implementation would generate something like:
> "e==1 || (e>=3 && e<=5)"
> Since the common case seems to be having large ranges of contiguous values, this is the difference between having N conditionals execute versus 2.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (THRIFT-223) Validate method should check that enum
types are assigned valid values
Posted by "Piotr Kozikowski (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/THRIFT-223?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Piotr Kozikowski updated THRIFT-223:
------------------------------------
Attachment: thrift-223.patch
This patch is a direct port of my first implementation for the ruby lib (THRIFT-224), since I already had the code. I'll probably change it to follow a different approach, as described in THRIFT-187, per Bryan's comment.
> Validate method should check that enum types are assigned valid values
> ----------------------------------------------------------------------
>
> Key: THRIFT-223
> URL: https://issues.apache.org/jira/browse/THRIFT-223
> Project: Thrift
> Issue Type: Improvement
> Components: Compiler (Java)
> Reporter: Nathan Marz
> Assignee: Piotr Kozikowski
> Priority: Minor
> Attachments: thrift-223.patch
>
>
> The validate method generated currently checks that required fields are set. It would be nice if it were to enforce more parts of the schema. One example of this are the values assigned to enum types. For example, if I have this enum:
> enum MyEnum {
> FOO = 1;
> BAR = 3;
> BAZ = 4;
> BIZ = 5;
> }
> and this struct:
> struct MyStruct {
> MyEnum e;
> }
> The validate method would ensure that MyStruct#e is either 1, 3, 4, or 5.
> The naive way of implementing this would be to generate a conditional statement for every value, aka
> "e==1 || e==3 || e==4 || e==5"
> A better implementation would generate something like:
> "e==1 || (e>=3 && e<=5)"
> Since the common case seems to be having large ranges of contiguous values, this is the difference between having N conditionals execute versus 2.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (THRIFT-223) Validate method should check that
enum types are assigned valid values
Posted by "Bryan Duxbury (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/THRIFT-223?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12657473#action_12657473 ]
Bryan Duxbury commented on THRIFT-223:
--------------------------------------
I don't think that we want each struct that uses a given enum to have the value set. I was hoping that the enum class itself would have the set, and the structs would just call out to that set. Does this make sense?
> Validate method should check that enum types are assigned valid values
> ----------------------------------------------------------------------
>
> Key: THRIFT-223
> URL: https://issues.apache.org/jira/browse/THRIFT-223
> Project: Thrift
> Issue Type: Improvement
> Components: Compiler (Java)
> Reporter: Nathan Marz
> Assignee: Piotr Kozikowski
> Priority: Minor
> Attachments: thrift-223-v2.patch, thrift-223.patch
>
>
> The validate method generated currently checks that required fields are set. It would be nice if it were to enforce more parts of the schema. One example of this are the values assigned to enum types. For example, if I have this enum:
> enum MyEnum {
> FOO = 1;
> BAR = 3;
> BAZ = 4;
> BIZ = 5;
> }
> and this struct:
> struct MyStruct {
> MyEnum e;
> }
> The validate method would ensure that MyStruct#e is either 1, 3, 4, or 5.
> The naive way of implementing this would be to generate a conditional statement for every value, aka
> "e==1 || e==3 || e==4 || e==5"
> A better implementation would generate something like:
> "e==1 || (e>=3 && e<=5)"
> Since the common case seems to be having large ranges of contiguous values, this is the difference between having N conditionals execute versus 2.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (THRIFT-223) Validate method should check that
enum types are assigned valid values
Posted by "David Reiss (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/THRIFT-223?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12660872#action_12660872 ]
David Reiss commented on THRIFT-223:
------------------------------------
This doesn't check the validity of enums that are in containers. Is that a problem?
Also, how come the "// alas, we cannot check" branch was removed? I think it is pretty important to have that case during serialization in non-beans style, because __isset is not automatically maintained.
> Validate method should check that enum types are assigned valid values
> ----------------------------------------------------------------------
>
> Key: THRIFT-223
> URL: https://issues.apache.org/jira/browse/THRIFT-223
> Project: Thrift
> Issue Type: Improvement
> Components: Compiler (Java)
> Reporter: Nathan Marz
> Assignee: Piotr Kozikowski
> Priority: Minor
> Attachments: thrift-223-v2.patch, thrift-223-v3.patch, thrift-223-v4.patch, thrift-223-v5.patch, thrift-223.patch
>
>
> The validate method generated currently checks that required fields are set. It would be nice if it were to enforce more parts of the schema. One example of this are the values assigned to enum types. For example, if I have this enum:
> enum MyEnum {
> FOO = 1;
> BAR = 3;
> BAZ = 4;
> BIZ = 5;
> }
> and this struct:
> struct MyStruct {
> MyEnum e;
> }
> The validate method would ensure that MyStruct#e is either 1, 3, 4, or 5.
> The naive way of implementing this would be to generate a conditional statement for every value, aka
> "e==1 || e==3 || e==4 || e==5"
> A better implementation would generate something like:
> "e==1 || (e>=3 && e<=5)"
> Since the common case seems to be having large ranges of contiguous values, this is the difference between having N conditionals execute versus 2.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Assigned: (THRIFT-223) Validate method should check that
enum types are assigned valid values
Posted by "Piotr Kozikowski (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/THRIFT-223?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Piotr Kozikowski reassigned THRIFT-223:
---------------------------------------
Assignee: Piotr Kozikowski
> Validate method should check that enum types are assigned valid values
> ----------------------------------------------------------------------
>
> Key: THRIFT-223
> URL: https://issues.apache.org/jira/browse/THRIFT-223
> Project: Thrift
> Issue Type: Improvement
> Components: Compiler (Java)
> Reporter: Nathan Marz
> Assignee: Piotr Kozikowski
> Priority: Minor
>
> The validate method generated currently checks that required fields are set. It would be nice if it were to enforce more parts of the schema. One example of this are the values assigned to enum types. For example, if I have this enum:
> enum MyEnum {
> FOO = 1;
> BAR = 3;
> BAZ = 4;
> BIZ = 5;
> }
> and this struct:
> struct MyStruct {
> MyEnum e;
> }
> The validate method would ensure that MyStruct#e is either 1, 3, 4, or 5.
> The naive way of implementing this would be to generate a conditional statement for every value, aka
> "e==1 || e==3 || e==4 || e==5"
> A better implementation would generate something like:
> "e==1 || (e>=3 && e<=5)"
> Since the common case seems to be having large ranges of contiguous values, this is the difference between having N conditionals execute versus 2.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (THRIFT-223) Validate method should check that enum
types are assigned valid values
Posted by "Bryan Duxbury (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/THRIFT-223?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Bryan Duxbury updated THRIFT-223:
---------------------------------
Patch Info: [Patch Available]
Except for the field-values-getter on generated structs, I think that this patch is ready to go.
> Validate method should check that enum types are assigned valid values
> ----------------------------------------------------------------------
>
> Key: THRIFT-223
> URL: https://issues.apache.org/jira/browse/THRIFT-223
> Project: Thrift
> Issue Type: Improvement
> Components: Compiler (Java)
> Reporter: Nathan Marz
> Assignee: Piotr Kozikowski
> Priority: Minor
> Attachments: thrift-223-v2.patch, thrift-223-v3.patch, thrift-223-v4.patch, thrift-223-v5.patch, thrift-223.patch
>
>
> The validate method generated currently checks that required fields are set. It would be nice if it were to enforce more parts of the schema. One example of this are the values assigned to enum types. For example, if I have this enum:
> enum MyEnum {
> FOO = 1;
> BAR = 3;
> BAZ = 4;
> BIZ = 5;
> }
> and this struct:
> struct MyStruct {
> MyEnum e;
> }
> The validate method would ensure that MyStruct#e is either 1, 3, 4, or 5.
> The naive way of implementing this would be to generate a conditional statement for every value, aka
> "e==1 || e==3 || e==4 || e==5"
> A better implementation would generate something like:
> "e==1 || (e>=3 && e<=5)"
> Since the common case seems to be having large ranges of contiguous values, this is the difference between having N conditionals execute versus 2.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Resolved: (THRIFT-223) Validate method should check that
enum types are assigned valid values
Posted by "Bryan Duxbury (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/THRIFT-223?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Bryan Duxbury resolved THRIFT-223.
----------------------------------
Resolution: Fixed
Committed in r735910. Thanks for the patch Piotr!
> Validate method should check that enum types are assigned valid values
> ----------------------------------------------------------------------
>
> Key: THRIFT-223
> URL: https://issues.apache.org/jira/browse/THRIFT-223
> Project: Thrift
> Issue Type: Improvement
> Components: Compiler (Java)
> Reporter: Nathan Marz
> Assignee: Piotr Kozikowski
> Priority: Minor
> Attachments: thrift-223-v2.patch, thrift-223-v3.patch, thrift-223-v4.patch, thrift-223-v5.patch, thrift-223-v6.patch, thrift-223.patch
>
>
> The validate method generated currently checks that required fields are set. It would be nice if it were to enforce more parts of the schema. One example of this are the values assigned to enum types. For example, if I have this enum:
> enum MyEnum {
> FOO = 1;
> BAR = 3;
> BAZ = 4;
> BIZ = 5;
> }
> and this struct:
> struct MyStruct {
> MyEnum e;
> }
> The validate method would ensure that MyStruct#e is either 1, 3, 4, or 5.
> The naive way of implementing this would be to generate a conditional statement for every value, aka
> "e==1 || e==3 || e==4 || e==5"
> A better implementation would generate something like:
> "e==1 || (e>=3 && e<=5)"
> Since the common case seems to be having large ranges of contiguous values, this is the difference between having N conditionals execute versus 2.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (THRIFT-223) Validate method should check that enum
types are assigned valid values
Posted by "Piotr Kozikowski (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/THRIFT-223?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Piotr Kozikowski updated THRIFT-223:
------------------------------------
Attachment: thrift-223-v3.patch
Duh, should have seen that coming. Here is the new version. Now every enum has a public constantsSet, and each field of type enum has a getFieldNameConstantsSet (or nocamel equivalent) method that returns that set.
> Validate method should check that enum types are assigned valid values
> ----------------------------------------------------------------------
>
> Key: THRIFT-223
> URL: https://issues.apache.org/jira/browse/THRIFT-223
> Project: Thrift
> Issue Type: Improvement
> Components: Compiler (Java)
> Reporter: Nathan Marz
> Assignee: Piotr Kozikowski
> Priority: Minor
> Attachments: thrift-223-v2.patch, thrift-223-v3.patch, thrift-223.patch
>
>
> The validate method generated currently checks that required fields are set. It would be nice if it were to enforce more parts of the schema. One example of this are the values assigned to enum types. For example, if I have this enum:
> enum MyEnum {
> FOO = 1;
> BAR = 3;
> BAZ = 4;
> BIZ = 5;
> }
> and this struct:
> struct MyStruct {
> MyEnum e;
> }
> The validate method would ensure that MyStruct#e is either 1, 3, 4, or 5.
> The naive way of implementing this would be to generate a conditional statement for every value, aka
> "e==1 || e==3 || e==4 || e==5"
> A better implementation would generate something like:
> "e==1 || (e>=3 && e<=5)"
> Since the common case seems to be having large ranges of contiguous values, this is the difference between having N conditionals execute versus 2.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (THRIFT-223) Validate method should check that enum
types are assigned valid values
Posted by "Piotr Kozikowski (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/THRIFT-223?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Piotr Kozikowski updated THRIFT-223:
------------------------------------
Attachment: thrift-223-v4.patch
I just realized there was a bug that caused the validate method to check for a valid enum value even for unset optional fields. Here is the fix.
> Validate method should check that enum types are assigned valid values
> ----------------------------------------------------------------------
>
> Key: THRIFT-223
> URL: https://issues.apache.org/jira/browse/THRIFT-223
> Project: Thrift
> Issue Type: Improvement
> Components: Compiler (Java)
> Reporter: Nathan Marz
> Assignee: Piotr Kozikowski
> Priority: Minor
> Attachments: thrift-223-v2.patch, thrift-223-v3.patch, thrift-223-v4.patch, thrift-223.patch
>
>
> The validate method generated currently checks that required fields are set. It would be nice if it were to enforce more parts of the schema. One example of this are the values assigned to enum types. For example, if I have this enum:
> enum MyEnum {
> FOO = 1;
> BAR = 3;
> BAZ = 4;
> BIZ = 5;
> }
> and this struct:
> struct MyStruct {
> MyEnum e;
> }
> The validate method would ensure that MyStruct#e is either 1, 3, 4, or 5.
> The naive way of implementing this would be to generate a conditional statement for every value, aka
> "e==1 || e==3 || e==4 || e==5"
> A better implementation would generate something like:
> "e==1 || (e>=3 && e<=5)"
> Since the common case seems to be having large ranges of contiguous values, this is the difference between having N conditionals execute versus 2.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.