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-224) Validate method should check that
enum types are assigned valid values
Validate method should check that enum types are assigned valid values
-----------------------------------------------------------------------
Key: THRIFT-224
URL: https://issues.apache.org/jira/browse/THRIFT-224
Project: Thrift
Issue Type: Improvement
Components: Compiler (Ruby)
Reporter: Nathan Marz
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-224) 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-224?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Piotr Kozikowski updated THRIFT-224:
------------------------------------
Attachment: thrift-224-v5.patch
I renamed CONSTANTS_SET to VALID_VALUES. The get_my_field_valid_values method is meant to make it easier to access the set, without having to know the name of the corresponding enum module. For example:
enum UnknownToMeEnum {
A = 1;
B = 1;
}
struct MyClass {
1: required UnknownToMeEnum my_field;
2: required string other_field;
}
x = MyClass.new
x.get_my_field_valid_values
Patch v5 doesn't have this method and patch v6 does in case you reconsider.
> Validate method should check that enum types are assigned valid values
> -----------------------------------------------------------------------
>
> Key: THRIFT-224
> URL: https://issues.apache.org/jira/browse/THRIFT-224
> Project: Thrift
> Issue Type: Improvement
> Components: Compiler (Ruby)
> Reporter: Nathan Marz
> Assignee: Piotr Kozikowski
> Attachments: thrift-224-v2.patch, thrift-224-v3.patch, thrift-224-v4.patch, thrift-224-v5.patch, thrift-224.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] Reopened: (THRIFT-224) 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-224?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Bryan Duxbury reopened THRIFT-224:
----------------------------------
Assignee: Piotr Kozikowski
Not a dupe - different library.
> Validate method should check that enum types are assigned valid values
> -----------------------------------------------------------------------
>
> Key: THRIFT-224
> URL: https://issues.apache.org/jira/browse/THRIFT-224
> Project: Thrift
> Issue Type: Improvement
> Components: Compiler (Ruby)
> Reporter: Nathan Marz
> Assignee: Piotr Kozikowski
>
> 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-224) 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-224?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Piotr Kozikowski updated THRIFT-224:
------------------------------------
Attachment: thrift-224-v2.patch
Here is a new patch that follows the approach you proposed. It's more elegant indeed. There is a catch now: the ruby freeze method doesn't work as one would expect with class Set:
set = Set.new([1,2])
set.freeze
set << 4
puts set.inspect
#<Set: {1, 2, 4}>
This happens because internally the set uses a hash, and it is that hash that should be frozen. Because of this users can alter the set of valid values for an enum type and thus compromise the validate method.
Do you know of any simple way to overcome this? (i.e. without having to redefine the class Set, etc.)
> Validate method should check that enum types are assigned valid values
> -----------------------------------------------------------------------
>
> Key: THRIFT-224
> URL: https://issues.apache.org/jira/browse/THRIFT-224
> Project: Thrift
> Issue Type: Improvement
> Components: Compiler (Ruby)
> Reporter: Nathan Marz
> Assignee: Piotr Kozikowski
> Attachments: thrift-224-v2.patch, thrift-224.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-224) 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-224?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Piotr Kozikowski updated THRIFT-224:
------------------------------------
Attachment: thrift-224-v6.patch
> Validate method should check that enum types are assigned valid values
> -----------------------------------------------------------------------
>
> Key: THRIFT-224
> URL: https://issues.apache.org/jira/browse/THRIFT-224
> Project: Thrift
> Issue Type: Improvement
> Components: Compiler (Ruby)
> Reporter: Nathan Marz
> Assignee: Piotr Kozikowski
> Attachments: thrift-224-v2.patch, thrift-224-v3.patch, thrift-224-v4.patch, thrift-224-v5.patch, thrift-224-v6.patch, thrift-224.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-224) 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-224?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12657424#action_12657424 ]
Bryan Duxbury commented on THRIFT-224:
--------------------------------------
I'm not really concerned that you can't freeze Ruby Set objects. If someone does something dumb like modifies the metadata structures on their object, that's their own problem.
> Validate method should check that enum types are assigned valid values
> -----------------------------------------------------------------------
>
> Key: THRIFT-224
> URL: https://issues.apache.org/jira/browse/THRIFT-224
> Project: Thrift
> Issue Type: Improvement
> Components: Compiler (Ruby)
> Reporter: Nathan Marz
> Assignee: Piotr Kozikowski
> Attachments: thrift-224-v2.patch, thrift-224.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-224) 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-224?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Piotr Kozikowski updated THRIFT-224:
------------------------------------
Attachment: thrift-224.patch
Duh! different library, of course. Here is a patch for the ruby generator.
> Validate method should check that enum types are assigned valid values
> -----------------------------------------------------------------------
>
> Key: THRIFT-224
> URL: https://issues.apache.org/jira/browse/THRIFT-224
> Project: Thrift
> Issue Type: Improvement
> Components: Compiler (Ruby)
> Reporter: Nathan Marz
> Assignee: Piotr Kozikowski
> Attachments: thrift-224.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-224) 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-224?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Bryan Duxbury updated THRIFT-224:
---------------------------------
Patch Info: [Patch Available]
> Validate method should check that enum types are assigned valid values
> -----------------------------------------------------------------------
>
> Key: THRIFT-224
> URL: https://issues.apache.org/jira/browse/THRIFT-224
> Project: Thrift
> Issue Type: Improvement
> Components: Compiler (Ruby)
> Reporter: Nathan Marz
> Assignee: Piotr Kozikowski
> Attachments: thrift-224-v2.patch, thrift-224-v3.patch, thrift-224-v4.patch, thrift-224-v5.patch, thrift-224-v6.patch, thrift-224.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-224) 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-224?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12656205#action_12656205 ]
Bryan Duxbury commented on THRIFT-224:
--------------------------------------
Interesting. I see you've implemented the "better" suggest from Nathan. Pretty cool.
I'm wondering if it would make more sense to implement THRIFT-187 and then use the set to do the validation. Instead of max(n) comparisons, you'd get exactly one hash and one comparison. It'd really be killing two birds with one stone.
> Validate method should check that enum types are assigned valid values
> -----------------------------------------------------------------------
>
> Key: THRIFT-224
> URL: https://issues.apache.org/jira/browse/THRIFT-224
> Project: Thrift
> Issue Type: Improvement
> Components: Compiler (Ruby)
> Reporter: Nathan Marz
> Assignee: Piotr Kozikowski
> Attachments: thrift-224.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-224) 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-224?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12659598#action_12659598 ]
Bryan Duxbury commented on THRIFT-224:
--------------------------------------
# Why does the generated struct need a get_my_field_constants_set method? Can't it just use MyField::CONSTANTS_SET? It seems like unnecessary code to generate.
# Instead of calling it CONSTANTS_SET, how about we make it POSSIBLE_VALUES or something like that? I don't want to confuse CONSTANTS_SET with the Ruby #constants method.
> Validate method should check that enum types are assigned valid values
> -----------------------------------------------------------------------
>
> Key: THRIFT-224
> URL: https://issues.apache.org/jira/browse/THRIFT-224
> Project: Thrift
> Issue Type: Improvement
> Components: Compiler (Ruby)
> Reporter: Nathan Marz
> Assignee: Piotr Kozikowski
> Attachments: thrift-224-v2.patch, thrift-224-v3.patch, thrift-224-v4.patch, thrift-224.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-224) 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-224?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Piotr Kozikowski updated THRIFT-224:
------------------------------------
Attachment: thrift-224-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-224
> URL: https://issues.apache.org/jira/browse/THRIFT-224
> Project: Thrift
> Issue Type: Improvement
> Components: Compiler (Ruby)
> Reporter: Nathan Marz
> Assignee: Piotr Kozikowski
> Attachments: thrift-224-v2.patch, thrift-224-v3.patch, thrift-224-v4.patch, thrift-224.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-224) 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-224?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Piotr Kozikowski resolved THRIFT-224.
-------------------------------------
Resolution: Duplicate
This is the same as THRIFT-223
> Validate method should check that enum types are assigned valid values
> -----------------------------------------------------------------------
>
> Key: THRIFT-224
> URL: https://issues.apache.org/jira/browse/THRIFT-224
> Project: Thrift
> Issue Type: Improvement
> Components: Compiler (Ruby)
> Reporter: Nathan Marz
>
> 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-224) 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-224?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12660997#action_12660997 ]
Bryan Duxbury commented on THRIFT-224:
--------------------------------------
Allow me to clarify. I think we should go with patch v5, the one that doesn't contain the get_my_field_valid_values implementation. I think this method doesn't add that much value, because either a) you know the enum class and you don't need the method or b) you're working with the field reflectively, and you don't know whether it's even an enum or not, so you wouldn't know to look for this method.
I think that the best thing to do is commit this patch without the extra method, and to implement THRIFT-245. Then you can find everything you need.
> Validate method should check that enum types are assigned valid values
> -----------------------------------------------------------------------
>
> Key: THRIFT-224
> URL: https://issues.apache.org/jira/browse/THRIFT-224
> Project: Thrift
> Issue Type: Improvement
> Components: Compiler (Ruby)
> Reporter: Nathan Marz
> Assignee: Piotr Kozikowski
> Attachments: thrift-224-v2.patch, thrift-224-v3.patch, thrift-224-v4.patch, thrift-224-v5.patch, thrift-224-v6.patch, thrift-224.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-224) 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-224?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Piotr Kozikowski updated THRIFT-224:
------------------------------------
Attachment: thrift-224-v3.patch
This new version of the patch places the set with the valid values of each enum as a constant inside the ruby module representing that enum. Classes have now a get_field_name_constants_set method for each field of type enum.
> Validate method should check that enum types are assigned valid values
> -----------------------------------------------------------------------
>
> Key: THRIFT-224
> URL: https://issues.apache.org/jira/browse/THRIFT-224
> Project: Thrift
> Issue Type: Improvement
> Components: Compiler (Ruby)
> Reporter: Nathan Marz
> Assignee: Piotr Kozikowski
> Attachments: thrift-224-v2.patch, thrift-224-v3.patch, thrift-224.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-224) 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-224?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12659854#action_12659854 ]
Bryan Duxbury commented on THRIFT-224:
--------------------------------------
I see where you're coming from, but I don't think that we need the extra method. The real deficiency is that the FIELDS constant in generated structs for enumerated fields don't contain the name of the enum they should draw values from. In any case, in order to consume the my_field_valid_values method, you'd already have to know that the field was an enumeration, and there's no way to do that, since FIELDS just records it as an i32.
> Validate method should check that enum types are assigned valid values
> -----------------------------------------------------------------------
>
> Key: THRIFT-224
> URL: https://issues.apache.org/jira/browse/THRIFT-224
> Project: Thrift
> Issue Type: Improvement
> Components: Compiler (Ruby)
> Reporter: Nathan Marz
> Assignee: Piotr Kozikowski
> Attachments: thrift-224-v2.patch, thrift-224-v3.patch, thrift-224-v4.patch, thrift-224-v5.patch, thrift-224-v6.patch, thrift-224.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-224) 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-224?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Bryan Duxbury resolved THRIFT-224.
----------------------------------
Resolution: Fixed
I just committed v5 with one small change. Instead of initializing VALID_VALUES with the constant values, I used the constant names. The result is functionally the same, but the generated code will look slightly prettier.
Thanks for the patch Piotr!
> Validate method should check that enum types are assigned valid values
> -----------------------------------------------------------------------
>
> Key: THRIFT-224
> URL: https://issues.apache.org/jira/browse/THRIFT-224
> Project: Thrift
> Issue Type: Improvement
> Components: Compiler (Ruby)
> Reporter: Nathan Marz
> Assignee: Piotr Kozikowski
> Attachments: thrift-224-v2.patch, thrift-224-v3.patch, thrift-224-v4.patch, thrift-224-v5.patch, thrift-224-v6.patch, thrift-224.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.