Repository: influxdata/influxdb
File: tsdb/shard.go (Go)

Code before:
			}
			validField = true
			break
		}
		if !validField {
			if reason == "" {
				reason = fmt.Sprintf(
					"invalid field name: input field \"%s\" on measurement \"%s\" is invalid",
					"time", string(p.Name()))
			}
			dropped++
			continue
		}

		// Skip any points whose keys have been dropped. Dropped has already been incremented for them.
		if len(droppedKeys) > 0 && bytesutil.Contains(droppedKeys, keys[i]) {
			continue
		}

		err := func(p models.Point, iter models.FieldIterator) error {
			var newFields []*FieldCreate
			var validateErr error
			name := p.Name()
			mf := engine.MeasurementFields(name)
			// Check with the field validator.
			if newFields, validateErr = ValidateFields(mf, p, s.options.Config.SkipFieldSizeValidation); validateErr != nil {
				var err PartialWriteError
				switch {
				case errors.As(validateErr, &err):
					// This will turn into an error later, outside this lambda
					if reason == "" {
						reason = err.Reason
					}
					dropped += err.Dropped
					atomic.AddInt64(&s.stats.WritePointsDropped, int64(err.Dropped))
				default:
					return err
				}
				return nil
			}

			points[j] = points[i]
			j++
			fieldsToCreate = append(fieldsToCreate, newFields...)
			return nil
		}(p, iter)
		if err != nil {
			return nil, nil, err
		}
	}
	if dropped > 0 {

Review comment (bug):
I know we're now in a mix of original code (before this PR and before #25998), but we are touching this and I now recommend another change for clarity, not correctness: removal of dead/unreachable code.

`ValidateFields()` _only_ returns a `PartialWriteError` at this point so the `switch { case errors.As(validateErr, &err) ...` is more than is needed as the `default:` case is unreachable code and therefore complicates the logic flow. 

My recommendation is to change `ValidateFields` to this signature (eliding args) so it returns the specific error it can only return:

`func ValidateFields(...) ([]*FieldCreate, *PartialWriteError)`, update it internally for &PartialWriteError and then update here to
```
var newFields []*FieldCreate
		var validateErr *PartialWriteError
		name := p.Name()
		mf := engine.MeasurementFields(name)
		// Check with the field validator.
		if newFields, validateErr = ValidateFields(mf, p, s.options.Config.SkipFieldSizeValidation); validateErr != nil {
			if reason == "" {
				// record the first reason only
				reason = validateErr.Reason
			}
			dropped += validateErr.Dropped
			atomic.AddInt64(&s.stats.WritePointsDropped, int64(validateErr.Dropped))
			continue
		}
```

I especially recommend this change since determining what to return in the default case was already tricky as this pr description notes that it: `Also fixes one bug in returning the wrong error.`

coda: 
I know this is slightly against the usual golang "return error interface in the ultimate return position" but in this case, for a helper method, I think it's clarifying instead of confusing: aka I find the switch with default more confusing than the momentary thought about why this helper doesn't return an `error` interface. 

Code after:
			}
			validField = true
			break
		}
		if !validField {
			if reason == "" {
				reason = fmt.Sprintf(
					"invalid field name: input field \"%s\" on measurement \"%s\" is invalid",
					"time", string(p.Name()))
			}
			dropped++
			continue
		}

		// Skip any points whose keys have been dropped. Dropped has already been incremented for them.
		if len(droppedKeys) > 0 && bytesutil.Contains(droppedKeys, keys[i]) {
			continue
		}

		var newFields []*FieldCreate
		var validateErr error
		name := p.Name()
		mf := engine.MeasurementFields(name)
		// Check with the field validator.
		if newFields, validateErr = ValidateFields(mf, p, s.options.Config.SkipFieldSizeValidation); validateErr != nil {
			var err PartialWriteError
			switch {
			case errors.As(validateErr, &err):
				// This will turn into an error later, outside this lambda
				if reason == "" {
					reason = err.Reason
				}
				dropped += err.Dropped
				atomic.AddInt64(&s.stats.WritePointsDropped, int64(err.Dropped))
				continue
			default:
				// Return validateErr, because err will be nil here
				return nil, nil, validateErr
			}
		}

		points[j] = points[i]
		j++
		fieldsToCreate = append(fieldsToCreate, newFields...)
	}
	if dropped > 0 {
		err = PartialWriteError{Reason: reason, Dropped: dropped, Database: s.database, RetentionPolicy: s.retentionPolicy}
	}

	return points[:j], fieldsToCreate, err
}