Skip to content

Commit 4119409

Browse files
authored
SQL: Improve validation of unsupported fields (#35675)
Fix bug in Analyzer that caused it to report unsupported fields only when declared in projections. The rule has been extended to all field declarations. Fix #35673
1 parent f8e333b commit 4119409

File tree

2 files changed

+146
-99
lines changed

2 files changed

+146
-99
lines changed

x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/analysis/analyzer/Analyzer.java

Lines changed: 31 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -153,8 +153,11 @@ private static <E extends Expression> E resolveExpression(E expression, LogicalP
153153
//
154154
// Shared methods around the analyzer rules
155155
//
156-
157156
private static Attribute resolveAgainstList(UnresolvedAttribute u, Collection<Attribute> attrList) {
157+
return resolveAgainstList(u, attrList, false);
158+
}
159+
160+
private static Attribute resolveAgainstList(UnresolvedAttribute u, Collection<Attribute> attrList, boolean allowCompound) {
158161
List<Attribute> matches = new ArrayList<>();
159162

160163
// first take into account the qualified version
@@ -181,7 +184,7 @@ private static Attribute resolveAgainstList(UnresolvedAttribute u, Collection<At
181184
}
182185

183186
if (matches.size() == 1) {
184-
return matches.get(0);
187+
return handleSpecialFields(u, matches.get(0), allowCompound);
185188
}
186189

187190
return u.withUnresolvedMessage("Reference [" + u.qualifiedName()
@@ -193,6 +196,25 @@ private static Attribute resolveAgainstList(UnresolvedAttribute u, Collection<At
193196
);
194197
}
195198

199+
private static Attribute handleSpecialFields(UnresolvedAttribute u, Attribute named, boolean allowCompound) {
200+
// if it's a object/compound type, keep it unresolved with a nice error message
201+
if (named instanceof FieldAttribute) {
202+
FieldAttribute fa = (FieldAttribute) named;
203+
// unsupported types
204+
if (DataTypes.isUnsupported(fa.dataType())) {
205+
UnsupportedEsField unsupportedField = (UnsupportedEsField) fa.field();
206+
named = u.withUnresolvedMessage(
207+
"Cannot use field [" + fa.name() + "] type [" + unsupportedField.getOriginalType() + "] as is unsupported");
208+
}
209+
// compound fields
210+
else if (allowCompound == false && fa.dataType().isPrimitive() == false) {
211+
named = u.withUnresolvedMessage(
212+
"Cannot use field [" + fa.name() + "] type [" + fa.dataType().esType + "] only its subfields");
213+
}
214+
}
215+
return named;
216+
}
217+
196218
private static boolean hasStar(List<? extends Expression> exprs) {
197219
for (Expression expression : exprs) {
198220
if (expression instanceof UnresolvedStar) {
@@ -348,21 +370,6 @@ else if (plan instanceof OrderBy) {
348370
NamedExpression named = resolveAgainstList(u, childrenOutput);
349371
// if resolved, return it; otherwise keep it in place to be resolved later
350372
if (named != null) {
351-
// if it's a object/compound type, keep it unresolved with a nice error message
352-
if (named instanceof FieldAttribute) {
353-
FieldAttribute fa = (FieldAttribute) named;
354-
if (DataTypes.isUnsupported(fa.dataType())) {
355-
UnsupportedEsField unsupportedField = (UnsupportedEsField) fa.field();
356-
named = u.withUnresolvedMessage(
357-
"Cannot use field [" + fa.name() + "] type [" + unsupportedField.getOriginalType() +
358-
"] as is unsupported");
359-
}
360-
else if (!fa.dataType().isPrimitive()) {
361-
named = u.withUnresolvedMessage(
362-
"Cannot use field [" + fa.name() + "] type [" + fa.dataType().esType + "] only its subfields");
363-
}
364-
}
365-
366373
if (log.isTraceEnabled()) {
367374
log.trace("Resolved {} to {}", u, named);
368375
}
@@ -407,15 +414,19 @@ private List<NamedExpression> expandStar(UnresolvedStar us, List<Attribute> outp
407414
if (us.qualifier() != null) {
408415
// resolve the so-called qualifier first
409416
// since this is an unresolved start we don't know whether it's a path or an actual qualifier
410-
Attribute q = resolveAgainstList(us.qualifier(), output);
417+
Attribute q = resolveAgainstList(us.qualifier(), output, true);
411418

412419
// the wildcard couldn't be expanded because the field doesn't exist at all
413420
// so, add to the list of expanded attributes its qualifier (the field without the wildcard)
414421
// the qualifier will be unresolved and later used in the error message presented to the user
415422
if (q == null) {
416-
expanded.add(us.qualifier());
417-
return expanded;
423+
return singletonList(us.qualifier());
424+
}
425+
// qualifier is unknown (e.g. unsupported type), bail out early
426+
else if (q.resolved() == false) {
427+
return singletonList(q);
418428
}
429+
419430
// now use the resolved 'qualifier' to match
420431
for (Attribute attr : output) {
421432
// filter the attributes that match based on their path

0 commit comments

Comments
 (0)