You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@drill.apache.org by Subbu Srinivasan <ss...@zscaler.com> on 2016/08/03 17:24:44 UTC
Re: [GitHub] drill issue #518: DRILL-4653.json - Malformed JSON
should not stop the entir...
Hi Folks,
When can we discuss this feature? Would next hangout be appropriate?
Thanks
Subbu
On Mon, Jul 25, 2016 at 10:20 AM, Subbu Srinivasan <ss...@zscaler.com>
wrote:
> This mechanism falls in line with other JSON processing similar to serde's
> with Hive, UDF's enabled at global level will apply to all users and is
> outlined using documentation.
>
>
> What is your stance if we move to the JSONFormatPlugin?
>
> On Fri, Jul 15, 2016 at 2:08 PM, jaltekruse <gi...@git.apache.org> wrote:
>
>> Github user jaltekruse commented on the issue:
>>
>> https://github.com/apache/drill/pull/518
>>
>> I don't think we should merge this without a mechanism to return a
>> warning to the user to tell them at least that some data was ignored, and
>> ideally some indication of how much data was discarded. While I do
>> understand this is not the default behavior, I think there is still too
>> high of a risk that an admin could set this at a global level and users
>> would be unaware of some of their data being discarded.
>>
>> I am willing to discuss the benefits of merging this before such a
>> system exists, but until this issue has been thoroughly evaluated I am -1
>> on the change.
>>
>> One improvement you could make to the current implementation is
>> moving the option to the format plugin instead of the system/session list.
>> This enables users to include setting the option in there query with the
>> "table with options" syntax that was added last fall. We already have a
>> JIRA open for moving the all_text_mode and read_numbers_as_double options
>> to this location, because it doesn't really make sense to change query
>> results based on session state. Unfortunately this change does not
>> completely remove my initial concern, because not all users can modify or
>> see the storage plugins in the case when web UI security is enabled.
>> Non-admin users in these cases could be surprised by this behavior.
>>
>> For examples of how this is done, you can look at the text plugin
>> config, you would just need to add these options as properties to the json
>> config which is currently mostly empty.
>>
>> https://github.com/apache/drill/blob/master/exec/java-exec/src/main/java/org/apache/drill/exec/store/easy/json/JSONFormatPlugin.java#L93
>>
>>
>> https://github.com/apache/drill/blob/master/exec/java-exec/src/main/java/org/apache/drill/exec/store/easy/text/TextFormatPlugin.java#L135
>>
>> Select with options: https://issues.apache.org/jira/browse/DRILL-4047
>> Jira for moving the existing options:
>> https://issues.apache.org/jira/browse/DRILL-4206
>>
>>
>> ---
>> If your project is set up for it, you can reply to this email and have
>> your
>> reply appear on GitHub as well. If your project does not have this feature
>> enabled and wishes so, or if the feature is enabled but not working,
>> please
>> contact infrastructure at infrastructure@apache.org or file a JIRA ticket
>> with INFRA.
>> ---
>>
>
>
Re: [GitHub] drill issue #518: DRILL-4653.json - Malformed JSON
should not stop the entir...
Posted by Jason Altekruse <ja...@dremio.com>.
Hey Parth and Subbu,
Sorry for missing the last message on this thread, I will be able to attend
the hangout tomorrow to discuss my concern. As I had said previously, I am
mostly trying to make sure that there is agreement about the impact of this
change on user behavior and expectations before we merge.
Jason Altekruse
Software Engineer at Dremio
Apache Drill Committer
On Thu, Aug 4, 2016 at 5:11 PM, Parth Chandra <pc...@maprtech.com> wrote:
> Hi Subbu,
>
> Yes we can discuss this on the next hangout. If Jason is able to attend
> we can discuss some way to address his concern.
>
> Parth
>
> On Wed, Aug 3, 2016 at 10:24 AM, Subbu Srinivasan <ssrinivasan@zscaler.com
> >
> wrote:
>
> > Hi Folks,
> > When can we discuss this feature? Would next hangout be appropriate?
> >
> > Thanks
> > Subbu
> >
> > On Mon, Jul 25, 2016 at 10:20 AM, Subbu Srinivasan <
> > ssrinivasan@zscaler.com>
> > wrote:
> >
> > > This mechanism falls in line with other JSON processing similar to
> > serde's
> > > with Hive, UDF's enabled at global level will apply to all users and is
> > > outlined using documentation.
> > >
> > >
> > > What is your stance if we move to the JSONFormatPlugin?
> > >
> > > On Fri, Jul 15, 2016 at 2:08 PM, jaltekruse <gi...@git.apache.org>
> wrote:
> > >
> > >> Github user jaltekruse commented on the issue:
> > >>
> > >> https://github.com/apache/drill/pull/518
> > >>
> > >> I don't think we should merge this without a mechanism to return a
> > >> warning to the user to tell them at least that some data was ignored,
> > and
> > >> ideally some indication of how much data was discarded. While I do
> > >> understand this is not the default behavior, I think there is still
> too
> > >> high of a risk that an admin could set this at a global level and
> users
> > >> would be unaware of some of their data being discarded.
> > >>
> > >> I am willing to discuss the benefits of merging this before such a
> > >> system exists, but until this issue has been thoroughly evaluated I am
> > -1
> > >> on the change.
> > >>
> > >> One improvement you could make to the current implementation is
> > >> moving the option to the format plugin instead of the system/session
> > list.
> > >> This enables users to include setting the option in there query with
> the
> > >> "table with options" syntax that was added last fall. We already have
> a
> > >> JIRA open for moving the all_text_mode and read_numbers_as_double
> > options
> > >> to this location, because it doesn't really make sense to change query
> > >> results based on session state. Unfortunately this change does not
> > >> completely remove my initial concern, because not all users can modify
> > or
> > >> see the storage plugins in the case when web UI security is enabled.
> > >> Non-admin users in these cases could be surprised by this behavior.
> > >>
> > >> For examples of how this is done, you can look at the text plugin
> > >> config, you would just need to add these options as properties to the
> > json
> > >> config which is currently mostly empty.
> > >>
> > >> https://github.com/apache/drill/blob/master/exec/java-
> > exec/src/main/java/org/apache/drill/exec/store/easy/json/
> > JSONFormatPlugin.java#L93
> > >>
> > >>
> > >> https://github.com/apache/drill/blob/master/exec/java-
> > exec/src/main/java/org/apache/drill/exec/store/easy/text/
> > TextFormatPlugin.java#L135
> > >>
> > >> Select with options: https://issues.apache.org/
> > jira/browse/DRILL-4047
> > >> Jira for moving the existing options:
> > >> https://issues.apache.org/jira/browse/DRILL-4206
> > >>
> > >>
> > >> ---
> > >> If your project is set up for it, you can reply to this email and have
> > >> your
> > >> reply appear on GitHub as well. If your project does not have this
> > feature
> > >> enabled and wishes so, or if the feature is enabled but not working,
> > >> please
> > >> contact infrastructure at infrastructure@apache.org or file a JIRA
> > ticket
> > >> with INFRA.
> > >> ---
> > >>
> > >
> > >
> >
>
Re: [GitHub] drill issue #518: DRILL-4653.json - Malformed JSON
should not stop the entir...
Posted by Parth Chandra <pc...@maprtech.com>.
Hi Subbu,
Yes we can discuss this on the next hangout. If Jason is able to attend
we can discuss some way to address his concern.
Parth
On Wed, Aug 3, 2016 at 10:24 AM, Subbu Srinivasan <ss...@zscaler.com>
wrote:
> Hi Folks,
> When can we discuss this feature? Would next hangout be appropriate?
>
> Thanks
> Subbu
>
> On Mon, Jul 25, 2016 at 10:20 AM, Subbu Srinivasan <
> ssrinivasan@zscaler.com>
> wrote:
>
> > This mechanism falls in line with other JSON processing similar to
> serde's
> > with Hive, UDF's enabled at global level will apply to all users and is
> > outlined using documentation.
> >
> >
> > What is your stance if we move to the JSONFormatPlugin?
> >
> > On Fri, Jul 15, 2016 at 2:08 PM, jaltekruse <gi...@git.apache.org> wrote:
> >
> >> Github user jaltekruse commented on the issue:
> >>
> >> https://github.com/apache/drill/pull/518
> >>
> >> I don't think we should merge this without a mechanism to return a
> >> warning to the user to tell them at least that some data was ignored,
> and
> >> ideally some indication of how much data was discarded. While I do
> >> understand this is not the default behavior, I think there is still too
> >> high of a risk that an admin could set this at a global level and users
> >> would be unaware of some of their data being discarded.
> >>
> >> I am willing to discuss the benefits of merging this before such a
> >> system exists, but until this issue has been thoroughly evaluated I am
> -1
> >> on the change.
> >>
> >> One improvement you could make to the current implementation is
> >> moving the option to the format plugin instead of the system/session
> list.
> >> This enables users to include setting the option in there query with the
> >> "table with options" syntax that was added last fall. We already have a
> >> JIRA open for moving the all_text_mode and read_numbers_as_double
> options
> >> to this location, because it doesn't really make sense to change query
> >> results based on session state. Unfortunately this change does not
> >> completely remove my initial concern, because not all users can modify
> or
> >> see the storage plugins in the case when web UI security is enabled.
> >> Non-admin users in these cases could be surprised by this behavior.
> >>
> >> For examples of how this is done, you can look at the text plugin
> >> config, you would just need to add these options as properties to the
> json
> >> config which is currently mostly empty.
> >>
> >> https://github.com/apache/drill/blob/master/exec/java-
> exec/src/main/java/org/apache/drill/exec/store/easy/json/
> JSONFormatPlugin.java#L93
> >>
> >>
> >> https://github.com/apache/drill/blob/master/exec/java-
> exec/src/main/java/org/apache/drill/exec/store/easy/text/
> TextFormatPlugin.java#L135
> >>
> >> Select with options: https://issues.apache.org/
> jira/browse/DRILL-4047
> >> Jira for moving the existing options:
> >> https://issues.apache.org/jira/browse/DRILL-4206
> >>
> >>
> >> ---
> >> If your project is set up for it, you can reply to this email and have
> >> your
> >> reply appear on GitHub as well. If your project does not have this
> feature
> >> enabled and wishes so, or if the feature is enabled but not working,
> >> please
> >> contact infrastructure at infrastructure@apache.org or file a JIRA
> ticket
> >> with INFRA.
> >> ---
> >>
> >
> >
>