You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by GitBox <gi...@apache.org> on 2019/02/21 22:21:24 UTC
[GitHub] HeartSaVioR commented on issue #23854: [SPARK-22000][SQL] Use
String.valueOf in generated code to assign value to String type of field in
Java Bean Encoder
HeartSaVioR commented on issue #23854: [SPARK-22000][SQL] Use String.valueOf in generated code to assign value to String type of field in Java Bean Encoder
URL: https://github.com/apache/spark/pull/23854#issuecomment-466190971
> Now there's a simple test case attached. Could we paste that in as a test here, and see if it even fails without the change? and if so, how.
The added UT was slightly modified from attached test case: I just modified it based on review comments. Previously added test failed without the change, and the revised test remains the same.
> I agree with your argument about null; it would never have worked here anyway. It feels like there should be an assertion about it to make sure, as if a null reaches here it returns "null" rather than an exception.
No I meant null has been handled well and modified code also works well with null. Please find `String.valueOf` from generated code - it is guarded with checking null. If the value is null, `String.valueOf` is not called and the result is null. That's what `propagateNull` does for us.
```
07:13:10.684 DEBUG org.apache.spark.sql.catalyst.expressions.codegen.GenerateSafeProjection: code for initializejavabean(newInstance(class test.org.apache.spark.sql.JavaBeanDeserializationSuite$RecordSpark22000), (setBooleanField,staticinvoke(class java.lang.String, ObjectType(class java.lang.String), valueOf, input[6, boolean, true], true, false)), (setLongField,staticinvoke(class java.lang.String, ObjectType(class java.lang.String), valueOf, input[2, bigint, true], true, false)), (setIntField,staticinvoke(class java.lang.String, ObjectType(class java.lang.String), valueOf, input[1, int, true], true, false)), (setStringField,staticinvoke(class java.lang.String, ObjectType(class java.lang.String), valueOf, input[5, string, true], true, false)), (setDoubleField,staticinvoke(class java.lang.String, ObjectType(class java.lang.String), valueOf, input[4, double, true], true, false)), (setShortField,staticinvoke(class java.lang.String, ObjectType(class java.lang.String), valueOf, input[0, smallint, true], true, false)), (setFloatField,staticinvoke(class java.lang.String, ObjectType(class java.lang.String), valueOf, input[3, float, true], true, false)), (setTimestampField,staticinvoke(class java.lang.String, ObjectType(class java.lang.String), valueOf, input[7, timestamp, true], true, false)), (setNullIntField,staticinvoke(class java.lang.String, ObjectType(class java.lang.String), valueOf, input[8, int, true], true, false))):
/* 001 */ public java.lang.Object generate(Object[] references) {
/* 002 */ return new SpecificSafeProjection(references);
/* 003 */ }
/* 004 */
/* 005 */ class SpecificSafeProjection extends org.apache.spark.sql.catalyst.expressions.codegen.BaseProjection {
/* 006 */
/* 007 */ private Object[] references;
/* 008 */ private InternalRow mutableRow;
/* 009 */ private boolean resultIsNull_0;
/* 010 */ private boolean argValue_0;
/* 011 */ private boolean resultIsNull_1;
/* 012 */ private long argValue_1;
/* 013 */ private boolean resultIsNull_2;
/* 014 */ private int argValue_2;
/* 015 */ private boolean resultIsNull_3;
/* 016 */ private boolean resultIsNull_4;
/* 017 */ private double argValue_3;
/* 018 */ private boolean resultIsNull_5;
/* 019 */ private short argValue_4;
/* 020 */ private boolean resultIsNull_6;
/* 021 */ private float argValue_5;
/* 022 */ private boolean resultIsNull_7;
/* 023 */ private long argValue_6;
/* 024 */ private boolean resultIsNull_8;
/* 025 */ private int argValue_7;
/* 026 */ private UTF8String[] mutableStateArray_0 = new UTF8String[1];
/* 027 */
/* 028 */ public SpecificSafeProjection(Object[] references) {
/* 029 */ this.references = references;
/* 030 */ mutableRow = (InternalRow) references[references.length - 1];
/* 031 */
/* 032 */
/* 033 */ }
/* 034 */
/* 035 */ public void initialize(int partitionIndex) {
/* 036 */
/* 037 */ }
/* 038 */
/* 039 */ public java.lang.Object apply(java.lang.Object _i) {
/* 040 */ InternalRow i = (InternalRow) _i;
/* 041 */ final test.org.apache.spark.sql.JavaBeanDeserializationSuite$RecordSpark22000 value_1 = false ?
/* 042 */ null : new test.org.apache.spark.sql.JavaBeanDeserializationSuite$RecordSpark22000();
/* 043 */ test.org.apache.spark.sql.JavaBeanDeserializationSuite$RecordSpark22000 javaBean_0 = value_1;
/* 044 */ if (!false) {
/* 045 */ initializeJavaBean_0_0(i, javaBean_0);
/* 046 */ initializeJavaBean_0_1(i, javaBean_0);
/* 047 */ initializeJavaBean_0_2(i, javaBean_0);
/* 048 */ }
/* 049 */ if (false) {
/* 050 */ mutableRow.setNullAt(0);
/* 051 */ } else {
/* 052 */
/* 053 */ mutableRow.update(0, value_1);
/* 054 */ }
/* 055 */
/* 056 */ return mutableRow;
/* 057 */ }
/* 058 */
/* 059 */
/* 060 */ private void initializeJavaBean_0_2(InternalRow i, test.org.apache.spark.sql.JavaBeanDeserializationSuite$RecordSpark22000 javaBean_0) {
/* 061 */
/* 062 */ resultIsNull_6 = false;
/* 063 */ if (!resultIsNull_6) {
/* 064 */ boolean isNull_15 = i.isNullAt(3);
/* 065 */ float value_15 = isNull_15 ?
/* 066 */ -1.0f : (i.getFloat(3));
/* 067 */ resultIsNull_6 = isNull_15;
/* 068 */ argValue_5 = value_15;
/* 069 */ }
/* 070 */
/* 071 */ boolean isNull_14 = resultIsNull_6;
/* 072 */ java.lang.String value_14 = null;
/* 073 */ if (!resultIsNull_6) {
/* 074 */ value_14 = java.lang.String.valueOf(argValue_5);
/* 075 */ }
/* 076 */ if (!isNull_14) {
/* 077 */ javaBean_0.setFloatField(value_14);
/* 078 */ }
/* 079 */
/* 080 */ resultIsNull_7 = false;
/* 081 */ if (!resultIsNull_7) {
/* 082 */ boolean isNull_17 = i.isNullAt(7);
/* 083 */ long value_17 = isNull_17 ?
/* 084 */ -1L : (i.getLong(7));
/* 085 */ resultIsNull_7 = isNull_17;
/* 086 */ argValue_6 = value_17;
/* 087 */ }
/* 088 */
/* 089 */ boolean isNull_16 = resultIsNull_7;
/* 090 */ java.lang.String value_16 = null;
/* 091 */ if (!resultIsNull_7) {
/* 092 */ value_16 = java.lang.String.valueOf(argValue_6);
/* 093 */ }
/* 094 */ if (!isNull_16) {
/* 095 */ javaBean_0.setTimestampField(value_16);
/* 096 */ }
/* 097 */
/* 098 */ resultIsNull_8 = false;
/* 099 */ if (!resultIsNull_8) {
/* 100 */ boolean isNull_19 = i.isNullAt(8);
/* 101 */ int value_19 = isNull_19 ?
/* 102 */ -1 : (i.getInt(8));
/* 103 */ resultIsNull_8 = isNull_19;
/* 104 */ argValue_7 = value_19;
/* 105 */ }
/* 106 */
/* 107 */ boolean isNull_18 = resultIsNull_8;
/* 108 */ java.lang.String value_18 = null;
/* 109 */ if (!resultIsNull_8) {
/* 110 */ value_18 = java.lang.String.valueOf(argValue_7);
/* 111 */ }
/* 112 */ if (!isNull_18) {
/* 113 */ javaBean_0.setNullIntField(value_18);
/* 114 */ }
/* 115 */
/* 116 */ }
/* 117 */
/* 118 */
/* 119 */ private void initializeJavaBean_0_1(InternalRow i, test.org.apache.spark.sql.JavaBeanDeserializationSuite$RecordSpark22000 javaBean_0) {
/* 120 */
/* 121 */ resultIsNull_3 = false;
/* 122 */ if (!resultIsNull_3) {
/* 123 */ boolean isNull_9 = i.isNullAt(5);
/* 124 */ UTF8String value_9 = isNull_9 ?
/* 125 */ null : (i.getUTF8String(5));
/* 126 */ resultIsNull_3 = isNull_9;
/* 127 */ mutableStateArray_0[0] = value_9;
/* 128 */ }
/* 129 */
/* 130 */ boolean isNull_8 = resultIsNull_3;
/* 131 */ java.lang.String value_8 = null;
/* 132 */ if (!resultIsNull_3) {
/* 133 */ value_8 = java.lang.String.valueOf(mutableStateArray_0[0]);
/* 134 */ }
/* 135 */ if (!isNull_8) {
/* 136 */ javaBean_0.setStringField(value_8);
/* 137 */ }
/* 138 */
/* 139 */ resultIsNull_4 = false;
/* 140 */ if (!resultIsNull_4) {
/* 141 */ boolean isNull_11 = i.isNullAt(4);
/* 142 */ double value_11 = isNull_11 ?
/* 143 */ -1.0 : (i.getDouble(4));
/* 144 */ resultIsNull_4 = isNull_11;
/* 145 */ argValue_3 = value_11;
/* 146 */ }
/* 147 */
/* 148 */ boolean isNull_10 = resultIsNull_4;
/* 149 */ java.lang.String value_10 = null;
/* 150 */ if (!resultIsNull_4) {
/* 151 */ value_10 = java.lang.String.valueOf(argValue_3);
/* 152 */ }
/* 153 */ if (!isNull_10) {
/* 154 */ javaBean_0.setDoubleField(value_10);
/* 155 */ }
/* 156 */
/* 157 */ resultIsNull_5 = false;
/* 158 */ if (!resultIsNull_5) {
/* 159 */ boolean isNull_13 = i.isNullAt(0);
/* 160 */ short value_13 = isNull_13 ?
/* 161 */ (short)-1 : (i.getShort(0));
/* 162 */ resultIsNull_5 = isNull_13;
/* 163 */ argValue_4 = value_13;
/* 164 */ }
/* 165 */
/* 166 */ boolean isNull_12 = resultIsNull_5;
/* 167 */ java.lang.String value_12 = null;
/* 168 */ if (!resultIsNull_5) {
/* 169 */ value_12 = java.lang.String.valueOf(argValue_4);
/* 170 */ }
/* 171 */ if (!isNull_12) {
/* 172 */ javaBean_0.setShortField(value_12);
/* 173 */ }
/* 174 */
/* 175 */ }
/* 176 */
/* 177 */
/* 178 */ private void initializeJavaBean_0_0(InternalRow i, test.org.apache.spark.sql.JavaBeanDeserializationSuite$RecordSpark22000 javaBean_0) {
/* 179 */
/* 180 */ resultIsNull_0 = false;
/* 181 */ if (!resultIsNull_0) {
/* 182 */ boolean isNull_3 = i.isNullAt(6);
/* 183 */ boolean value_3 = isNull_3 ?
/* 184 */ false : (i.getBoolean(6));
/* 185 */ resultIsNull_0 = isNull_3;
/* 186 */ argValue_0 = value_3;
/* 187 */ }
/* 188 */
/* 189 */ boolean isNull_2 = resultIsNull_0;
/* 190 */ java.lang.String value_2 = null;
/* 191 */ if (!resultIsNull_0) {
/* 192 */ value_2 = java.lang.String.valueOf(argValue_0);
/* 193 */ }
/* 194 */ if (!isNull_2) {
/* 195 */ javaBean_0.setBooleanField(value_2);
/* 196 */ }
/* 197 */
/* 198 */ resultIsNull_1 = false;
/* 199 */ if (!resultIsNull_1) {
/* 200 */ boolean isNull_5 = i.isNullAt(2);
/* 201 */ long value_5 = isNull_5 ?
/* 202 */ -1L : (i.getLong(2));
/* 203 */ resultIsNull_1 = isNull_5;
/* 204 */ argValue_1 = value_5;
/* 205 */ }
/* 206 */
/* 207 */ boolean isNull_4 = resultIsNull_1;
/* 208 */ java.lang.String value_4 = null;
/* 209 */ if (!resultIsNull_1) {
/* 210 */ value_4 = java.lang.String.valueOf(argValue_1);
/* 211 */ }
/* 212 */ if (!isNull_4) {
/* 213 */ javaBean_0.setLongField(value_4);
/* 214 */ }
/* 215 */
/* 216 */ resultIsNull_2 = false;
/* 217 */ if (!resultIsNull_2) {
/* 218 */ boolean isNull_7 = i.isNullAt(1);
/* 219 */ int value_7 = isNull_7 ?
/* 220 */ -1 : (i.getInt(1));
/* 221 */ resultIsNull_2 = isNull_7;
/* 222 */ argValue_2 = value_7;
/* 223 */ }
/* 224 */
/* 225 */ boolean isNull_6 = resultIsNull_2;
/* 226 */ java.lang.String value_6 = null;
/* 227 */ if (!resultIsNull_2) {
/* 228 */ value_6 = java.lang.String.valueOf(argValue_2);
/* 229 */ }
/* 230 */ if (!isNull_6) {
/* 231 */ javaBean_0.setIntField(value_6);
/* 232 */ }
/* 233 */
/* 234 */ }
/* 235 */
/* 236 */ }
```
So it's completely safe to use `String.valueOf()` for String type, even other types in if statement.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org