exceptionfactory commented on code in PR #9379:
URL: https://github.com/apache/nifi/pull/9379#discussion_r1799701807


##########
nifi-extension-bundles/nifi-poi-bundle/nifi-poi-services/src/main/java/org/apache/nifi/excel/StandardCellFieldTypeReader.java:
##########
@@ -0,0 +1,129 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.nifi.excel;
+
+import org.apache.nifi.schema.inference.FieldTypeInference;
+import org.apache.nifi.schema.inference.TimeValueInference;
+import org.apache.nifi.serialization.record.DataType;
+import org.apache.nifi.serialization.record.RecordFieldType;
+import org.apache.poi.ss.usermodel.Cell;
+import org.apache.poi.ss.usermodel.CellType;
+import org.apache.poi.ss.usermodel.DateUtil;
+
+import java.util.Map;
+import java.util.Objects;
+import java.util.Optional;
+
+/**
+ * Standard implementation of Cell Field Type Reader uses Cell Type and Cell 
Value information with inference based on Timestamp formats
+ */
+class StandardCellFieldTypeReader implements CellFieldTypeReader {
+    private final TimeValueInference timeValueInference;
+
+    StandardCellFieldTypeReader(final TimeValueInference timeValueInference) {
+        this.timeValueInference = Objects.requireNonNull(timeValueInference, 
"Time Value Inference required");
+    }
+
+    /**
+     * Infer Cell Field Type and append possible Data Types to mapped Field 
Type Inference information
+     *
+     * @param cell Spreadsheet Cell required
+     * @param fieldName Cell field name for tracking in Field Types
+     * @param fieldTypes Map of Field Name to Field Type Inference information
+     */
+    @Override
+    public void inferCellFieldType(final Cell cell, final String fieldName, 
final Map<String, FieldTypeInference> fieldTypes) {
+        Objects.requireNonNull(cell, "Cell required");
+        Objects.requireNonNull(fieldName, "Field Name required");
+        Objects.requireNonNull(fieldTypes, "Field Types required");
+
+        final FieldTypeInference fieldTypeInference = 
fieldTypes.computeIfAbsent(fieldName, key -> new FieldTypeInference());
+        final DataType dataType = getCellDataType(cell);
+        fieldTypeInference.addPossibleDataType(dataType);
+    }
+
+    /**
+     * Get Record Data Type from Spreadsheet Cell Type and additional 
resolution of NUMERIC and STRING types
+     *
+     * @param cell Spreadsheet Cell required
+     * @return Record Data Type
+     */
+    @Override
+    public DataType getCellDataType(final Cell cell) {
+        Objects.requireNonNull(cell, "Cell required");
+
+        final CellType cellType = cell.getCellType();
+
+        final DataType dataType;
+
+        if (CellType.NUMERIC == cellType) {
+            // Date Formatting check limited to NUMERIC Cell Types
+            if (DateUtil.isCellDateFormatted(cell)) {
+                // Default to TIMESTAMP for date formatted values using 
standard Date Time Pattern
+                dataType = RecordFieldType.TIMESTAMP.getDataType();
+            } else {
+                final double numericCellValue = cell.getNumericCellValue();
+                final long roundedCellValue = (long) numericCellValue;
+                if (roundedCellValue == numericCellValue) {
+                    dataType = RecordFieldType.LONG.getDataType();
+                } else {
+                    // Default to DOUBLE for NUMERIC values following 
cell.getNumericCellValue()
+                    dataType = RecordFieldType.DOUBLE.getDataType();
+                }
+            }
+        } else if (CellType.BOOLEAN == cellType) {
+            dataType = RecordFieldType.BOOLEAN.getDataType();
+        } else if (CellType.STRING == cellType) {
+            final String cellValue = cell.getStringCellValue();
+            // Attempt Time Value inference for STRING cell values
+            final Optional<DataType> timeDataType = 
timeValueInference.getDataType(cellValue);

Review Comment:
   That's a good question. It depends how flexible we should be for timestamp 
handling. In other words, it is possible for a `STRING` cell to contain 
something that looks like a timestamp. This would effectively remove the need 
for any time or date format properties in the `ExcelReader`, but it would 
require times and dates to be stored as `NUMERIC` types. I'm inclined to leave 
it in, given the existence of those format properties, but it might be helpful 
to keep it for now to enable wider time and date detection from some 
spreadsheets.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to