[jira] Commented: (SOLR-1591) XMLWriter#writeAttr silently ignores null attribute values
[
https://issues.apache.org/jira/browse/SOLR-1591?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12799792#action_12799792
]
Chris A. Mattmann commented on SOLR-1591:
-
bq. Why are you arguing performance if you're going to look up the key twice?
It's an order of magnitude more expensive.
Just copied it through from your example. Your point was to show it was a bit
messier to do the check and my point was to show it's all about how you
organize that messiness.
bq. This is such a trivial issue though... we really shouldn't be wasting
breath on it.
Fantastic point. You threw up a suggestion:
{quote}
And if we moved the null check to the callers, I'd argue that the null check
should be entirely left out of writeAttr - skip the extra code and let the NPE
happen naturally.
{quote}
That sounds like a plan, +1 to that. Then, we can stop wasting time on this and
move on to (more) interesting stuff...
> XMLWriter#writeAttr silently ignores null attribute values
> --
>
> Key: SOLR-1591
> URL: https://issues.apache.org/jira/browse/SOLR-1591
> Project: Solr
> Issue Type: Improvement
>Affects Versions: 1.1.0
> Environment: My local MacBook pro laptop.
>Reporter: Chris A. Mattmann
>Priority: Minor
> Attachments: SOLR-1591.Mattmann.112209.patch.txt
>
>
> XMLWriter#writeAttr checks for val == null, and if so, does nothing. Instead
> of doing nothing, it could leverage its method signature, and throw an
> IOException declaring that the value provided is null. Patch, attached.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (SOLR-1591) XMLWriter#writeAttr silently ignores null attribute values
[
https://issues.apache.org/jira/browse/SOLR-1591?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12799787#action_12799787
]
Yonik Seeley commented on SOLR-1591:
{quote}
But what about this?
if(map.get("foo")) != null writeAttr("foo", map.get("foo"));
if(map.get("bar")) != null writeAttr("bar", map.get("bar"));
{quote}
Why are you arguing performance if you're going to look up the key twice? It's
an order of magnitude more expensive.
This is such a trivial issue though... we really shouldn't be wasting breath on
it.
> XMLWriter#writeAttr silently ignores null attribute values
> --
>
> Key: SOLR-1591
> URL: https://issues.apache.org/jira/browse/SOLR-1591
> Project: Solr
> Issue Type: Bug
>Affects Versions: 1.1.0
> Environment: My local MacBook pro laptop.
>Reporter: Chris A. Mattmann
>Priority: Minor
> Attachments: SOLR-1591.Mattmann.112209.patch.txt
>
>
> XMLWriter#writeAttr checks for val == null, and if so, does nothing. Instead
> of doing nothing, it could leverage its method signature, and throw an
> IOException declaring that the value provided is null. Patch, attached.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (SOLR-1591) XMLWriter#writeAttr silently ignores null attribute values
[
https://issues.apache.org/jira/browse/SOLR-1591?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12799784#action_12799784
]
Chris A. Mattmann commented on SOLR-1591:
-
{quote}
shrugs - the example I gave was meant to show how defining the function to work
for "null" can simplify code that calls the function. What didn't you
understand about that?
{quote}
Well I understood your comment I was just confused as to how it's any better I
guess. You showed this as a simplification that having the silently ignored
null allows:
{code}
writeAttr("foo", map.get("foo"));
writeAttr("foo", map.get("bar"));
{code}
Agreed, it's a bit prettier than this:
{code}
String fooVal = map.get("foo");
if (fooVal != null) {
writeAttr("foo", fooVal);
}
String barVal= map.get("bar");
if (barVal!= null) {
writeAttr("bar", barVal);
}
...
{code}
But what about this?
{code}
if(map.get("foo")) != null writeAttr("foo", map.get("foo"));
if(map.get("bar")) != null writeAttr("bar", map.get("bar"));
{code}
{quote}
No, not if the function is well defined for a null value.
{quote}
It can be as well defined as you'd like but in the end it's a no-op and a
waste, and more importantly it's a silent waste, right?
{quote}
And if we moved the null check to the callers, I'd argue that the null check
should be entirely left out of writeAttr - skip the extra code and let the NPE
happen naturally.
{quote}
+1 to this. What do you think?
Cheers,
Chris
> XMLWriter#writeAttr silently ignores null attribute values
> --
>
> Key: SOLR-1591
> URL: https://issues.apache.org/jira/browse/SOLR-1591
> Project: Solr
> Issue Type: Bug
>Affects Versions: 1.4
> Environment: My local MacBook pro laptop.
>Reporter: Chris A. Mattmann
>Priority: Minor
> Fix For: 1.5
>
> Attachments: SOLR-1591.Mattmann.112209.patch.txt
>
>
> XMLWriter#writeAttr checks for val == null, and if so, does nothing. Instead
> of doing nothing, it could leverage its method signature, and throw an
> IOException declaring that the value provided is null. Patch, attached.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (SOLR-1591) XMLWriter#writeAttr silently ignores null attribute values
[ https://issues.apache.org/jira/browse/SOLR-1591?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12799779#action_12799779 ] Yonik Seeley commented on SOLR-1591: bq. Thanks for your last comment, but I'm still confused. *shrugs* - the example I gave was meant to show how defining the function to work for "null" can simplify code that calls the function. What didn't you understand about that? bq. At the very least, the method should log a message saying "a null attribute was attempted to be written". No, not if the function is well defined for a null value. And if we moved the null check to the callers, I'd argue that the null check should be entirely left out of writeAttr - skip the extra code and let the NPE happen naturally. > XMLWriter#writeAttr silently ignores null attribute values > -- > > Key: SOLR-1591 > URL: https://issues.apache.org/jira/browse/SOLR-1591 > Project: Solr > Issue Type: Bug >Affects Versions: 1.4 > Environment: My local MacBook pro laptop. >Reporter: Chris A. Mattmann >Priority: Minor > Fix For: 1.5 > > Attachments: SOLR-1591.Mattmann.112209.patch.txt > > > XMLWriter#writeAttr checks for val == null, and if so, does nothing. Instead > of doing nothing, it could leverage its method signature, and throw an > IOException declaring that the value provided is null. Patch, attached. -- This message is automatically generated by JIRA. - You can reply to this email to add a comment to the issue online.
[jira] Commented: (SOLR-1591) XMLWriter#writeAttr silently ignores null attribute values
[
https://issues.apache.org/jira/browse/SOLR-1591?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12799628#action_12799628
]
Chris A. Mattmann commented on SOLR-1591:
-
Hey Noble:
Here are the disadvantages:
{quote}
* null values are rare .optimizing for cornercases is not a good idea
{quote}
It's not a corner case: much of the SOLR XML response calls this function for a
null value (e.g., any tag without an explicit "name" attribute, which are e.g.,
all the docs in a result set, which can be large, depending on the start/end
row params.
{quote}
* the cost of a function call is almost zilch. it is not even measurable
{quote}
cost(function) > cost(comparison)
> XMLWriter#writeAttr silently ignores null attribute values
> --
>
> Key: SOLR-1591
> URL: https://issues.apache.org/jira/browse/SOLR-1591
> Project: Solr
> Issue Type: Bug
>Affects Versions: 1.4
> Environment: My local MacBook pro laptop.
>Reporter: Chris A. Mattmann
>Priority: Minor
> Fix For: 1.5
>
> Attachments: SOLR-1591.Mattmann.112209.patch.txt
>
>
> XMLWriter#writeAttr checks for val == null, and if so, does nothing. Instead
> of doing nothing, it could leverage its method signature, and throw an
> IOException declaring that the value provided is null. Patch, attached.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (SOLR-1591) XMLWriter#writeAttr silently ignores null attribute values
[ https://issues.apache.org/jira/browse/SOLR-1591?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12799627#action_12799627 ] Noble Paul commented on SOLR-1591: -- bq.Why waste a function call here? * null values are rare .optimizing for cornercases is not a good idea * the cost of a function call is almost zilch. it is not even measurable > XMLWriter#writeAttr silently ignores null attribute values > -- > > Key: SOLR-1591 > URL: https://issues.apache.org/jira/browse/SOLR-1591 > Project: Solr > Issue Type: Bug >Affects Versions: 1.4 > Environment: My local MacBook pro laptop. >Reporter: Chris A. Mattmann >Priority: Minor > Fix For: 1.5 > > Attachments: SOLR-1591.Mattmann.112209.patch.txt > > > XMLWriter#writeAttr checks for val == null, and if so, does nothing. Instead > of doing nothing, it could leverage its method signature, and throw an > IOException declaring that the value provided is null. Patch, attached. -- This message is automatically generated by JIRA. - You can reply to this email to add a comment to the issue online.
[jira] Commented: (SOLR-1591) XMLWriter#writeAttr silently ignores null attribute values
[ https://issues.apache.org/jira/browse/SOLR-1591?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12799621#action_12799621 ] Chris A. Mattmann commented on SOLR-1591: - It may work as designed Noble, but as designed, it's a wasted function call. SOLR is all about performance. Why waste a function call here? > XMLWriter#writeAttr silently ignores null attribute values > -- > > Key: SOLR-1591 > URL: https://issues.apache.org/jira/browse/SOLR-1591 > Project: Solr > Issue Type: Bug >Affects Versions: 1.4 > Environment: My local MacBook pro laptop. >Reporter: Chris A. Mattmann >Priority: Minor > Fix For: 1.5 > > Attachments: SOLR-1591.Mattmann.112209.patch.txt > > > XMLWriter#writeAttr checks for val == null, and if so, does nothing. Instead > of doing nothing, it could leverage its method signature, and throw an > IOException declaring that the value provided is null. Patch, attached. -- This message is automatically generated by JIRA. - You can reply to this email to add a comment to the issue online.
[jira] Commented: (SOLR-1591) XMLWriter#writeAttr silently ignores null attribute values
[ https://issues.apache.org/jira/browse/SOLR-1591?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12799617#action_12799617 ] Noble Paul commented on SOLR-1591: -- bq.I'd favor an exception to be thrown not required. This works as designed. we just need enough javadocs to tell it clearly. > XMLWriter#writeAttr silently ignores null attribute values > -- > > Key: SOLR-1591 > URL: https://issues.apache.org/jira/browse/SOLR-1591 > Project: Solr > Issue Type: Bug >Affects Versions: 1.4 > Environment: My local MacBook pro laptop. >Reporter: Chris A. Mattmann >Priority: Minor > Fix For: 1.5 > > Attachments: SOLR-1591.Mattmann.112209.patch.txt > > > XMLWriter#writeAttr checks for val == null, and if so, does nothing. Instead > of doing nothing, it could leverage its method signature, and throw an > IOException declaring that the value provided is null. Patch, attached. -- This message is automatically generated by JIRA. - You can reply to this email to add a comment to the issue online.
[jira] Commented: (SOLR-1591) XMLWriter#writeAttr silently ignores null attribute values
[ https://issues.apache.org/jira/browse/SOLR-1591?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12799604#action_12799604 ] Chris A. Mattmann commented on SOLR-1591: - Hey Yonik: Thanks for your last comment, but I'm still confused. This is a wasted method call. At the very least, the method should log a message saying "a null attribute was attempted to be written". Otherwise you have no record that the function was even called at all, which makes it hard to debug and track things down. I'd favor an exception to be thrown and a null check explicitly (at the expense of the additional lines of code) since it's more verbose and explicit rather than implicit. Thoughts? Anyone? Cheers, Chris > XMLWriter#writeAttr silently ignores null attribute values > -- > > Key: SOLR-1591 > URL: https://issues.apache.org/jira/browse/SOLR-1591 > Project: Solr > Issue Type: Bug >Affects Versions: 1.4 > Environment: My local MacBook pro laptop. >Reporter: Chris A. Mattmann >Priority: Minor > Fix For: 1.5 > > Attachments: SOLR-1591.Mattmann.112209.patch.txt > > > XMLWriter#writeAttr checks for val == null, and if so, does nothing. Instead > of doing nothing, it could leverage its method signature, and throw an > IOException declaring that the value provided is null. Patch, attached. -- This message is automatically generated by JIRA. - You can reply to this email to add a comment to the issue online.
[jira] Commented: (SOLR-1591) XMLWriter#writeAttr silently ignores null attribute values
[
https://issues.apache.org/jira/browse/SOLR-1591?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12781298#action_12781298
]
Yonik Seeley commented on SOLR-1591:
Implementing a null check in one place can be better than implementing it in
many.
{code}
String fooVal = map.get("foo");
if (fooVal != null) {
writeAttr("foo", fooVal);
}
String barVal= map.get("bar");
if (barVal!= null) {
writeAttr("bar", barVal);
}
...
{code}
vs
{code}
writeAttr("foo", map.get("foo"));
writeAttr("foo", map.get("bar"));
...
{code}
I've added the following javadoc to clarify:
/** Writes the XML attribute name/val. A null val means that the attribute is
missing */
> XMLWriter#writeAttr silently ignores null attribute values
> --
>
> Key: SOLR-1591
> URL: https://issues.apache.org/jira/browse/SOLR-1591
> Project: Solr
> Issue Type: Bug
>Affects Versions: 1.4
> Environment: My local MacBook pro laptop.
>Reporter: Chris A. Mattmann
>Priority: Minor
> Fix For: 1.5
>
> Attachments: SOLR-1591.Mattmann.112209.patch.txt
>
>
> XMLWriter#writeAttr checks for val == null, and if so, does nothing. Instead
> of doing nothing, it could leverage its method signature, and throw an
> IOException declaring that the value provided is null. Patch, attached.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (SOLR-1591) XMLWriter#writeAttr silently ignores null attribute values
[ https://issues.apache.org/jira/browse/SOLR-1591?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12781294#action_12781294 ] Chris A. Mattmann commented on SOLR-1591: - I don't get it. If someone passes in a null attribute, this method does nothing. It simply returns. What's the point? You don't get any feedback it's null in that case even and it just wastes a function call? I actually ran into this while trying to write XML output for this georss stuff (accidentally passed in a null ns attr and SOLR was all quiet on the western front...) > XMLWriter#writeAttr silently ignores null attribute values > -- > > Key: SOLR-1591 > URL: https://issues.apache.org/jira/browse/SOLR-1591 > Project: Solr > Issue Type: Bug >Affects Versions: 1.4 > Environment: My local MacBook pro laptop. >Reporter: Chris A. Mattmann >Priority: Minor > Fix For: 1.5 > > Attachments: SOLR-1591.Mattmann.112209.patch.txt > > > XMLWriter#writeAttr checks for val == null, and if so, does nothing. Instead > of doing nothing, it could leverage its method signature, and throw an > IOException declaring that the value provided is null. Patch, attached. -- This message is automatically generated by JIRA. - You can reply to this email to add a comment to the issue online.
[jira] Commented: (SOLR-1591) XMLWriter#writeAttr silently ignores null attribute values
[ https://issues.apache.org/jira/browse/SOLR-1591?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12781293#action_12781293 ] Yonik Seeley commented on SOLR-1591: This actually changes the functionality though... one could also think of it as "null" being a valid attribute value to pass, and the encoding of that shall be to leave it out entirely. I don't really know/remember if it's used that way or not, but it's safest to assume that it is. > XMLWriter#writeAttr silently ignores null attribute values > -- > > Key: SOLR-1591 > URL: https://issues.apache.org/jira/browse/SOLR-1591 > Project: Solr > Issue Type: Bug >Affects Versions: 1.4 > Environment: My local MacBook pro laptop. >Reporter: Chris A. Mattmann >Priority: Minor > Fix For: 1.5 > > Attachments: SOLR-1591.Mattmann.112209.patch.txt > > > XMLWriter#writeAttr checks for val == null, and if so, does nothing. Instead > of doing nothing, it could leverage its method signature, and throw an > IOException declaring that the value provided is null. Patch, attached. -- This message is automatically generated by JIRA. - You can reply to this email to add a comment to the issue online.
