Skip to content

Commit

Permalink
refactor: remove default cases over YBCPgDataType
Browse files Browse the repository at this point in the history 
Summary:
Default cases in switch over YBCPgDataType should not exist.  When a new
type is created, if there is no default case, a compilation error would
be thrown so that the author can take a look at the switch and handle it
manually.  Default cases don't throw compilation error, so a potential
bug is more difficult to trace because it may only manifest in runtime.

For now, get rid of the default cases for switches on YBCPgDataType
only.  Don't worry about the other switches.

Second, define YB_PG_UNSUPPORTED_TYPES_IN_SWITCH and
YB_PG_INVALID_TYPES_IN_SWITCH to reduce duplicate code.

Test Plan: jenkins

Reviewers: mihnea, neil, smishra

Reviewed By: smishra

Subscribers: yql, timur

Differential Revision: https://phabricator.dev.yugabyte.com/D12646
  • Loading branch information
@jaki
jaki committed Aug 31, 2021
1 parent a057a2a commit e130615
Show file tree
Hide file tree
Showing 4 changed files with 38 additions and 69 deletions.
2 changes: 1 addition & 1 deletion src/yb/common/ql_value.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ using yb::operator"" _MB;
DEFINE_int32(yql_max_value_size, 64_MB,
"Maximum size of a value in the Yugabyte Query Layer");

// The list of unsupported datypes to use in switch statements
// The list of unsupported datatypes to use in switch statements
#define QL_UNSUPPORTED_TYPES_IN_SWITCH \
case NULL_VALUE_TYPE: FALLTHROUGH_INTENDED; \
case TUPLE: FALLTHROUGH_INTENDED; \
Expand Down
39 changes: 5 additions & 34 deletions src/yb/yql/pggate/pg_expr.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
#include "yb/common/pg_system_attr.h"
#include "yb/yql/pggate/pg_expr.h"
#include "yb/yql/pggate/pg_dml.h"
#include "yb/yql/pggate/ybc_pg_typedefs.h"
#include "yb/util/string_util.h"
#include "yb/util/decimal.h"
#include "yb/util/flag_tags.h"
Expand Down Expand Up @@ -351,23 +352,8 @@ void PgExpr::InitializeTranslateData() {
translate_data_ = TranslateDecimal;
break;

case YB_YQL_DATA_TYPE_VARINT:
case YB_YQL_DATA_TYPE_INET:
case YB_YQL_DATA_TYPE_LIST:
case YB_YQL_DATA_TYPE_MAP:
case YB_YQL_DATA_TYPE_SET:
case YB_YQL_DATA_TYPE_UUID:
case YB_YQL_DATA_TYPE_TIMEUUID:
case YB_YQL_DATA_TYPE_TUPLE:
case YB_YQL_DATA_TYPE_TYPEARGS:
case YB_YQL_DATA_TYPE_USER_DEFINED_TYPE:
case YB_YQL_DATA_TYPE_FROZEN:
case YB_YQL_DATA_TYPE_DATE: // Not used for PG storage
case YB_YQL_DATA_TYPE_TIME: // Not used for PG storage
case YB_YQL_DATA_TYPE_JSONB:
case YB_YQL_DATA_TYPE_UINT8:
case YB_YQL_DATA_TYPE_UINT16:
default:
YB_PG_UNSUPPORTED_TYPES_IN_SWITCH:
YB_PG_INVALID_TYPES_IN_SWITCH:
LOG(DFATAL) << "Internal error: unsupported type " << type_entity_->yb_type;
}
}
Expand Down Expand Up @@ -493,23 +479,8 @@ PgConstant::PgConstant(const YBCPgTypeEntity *type_entity, uint64_t datum, bool
}
break;

case YB_YQL_DATA_TYPE_VARINT:
case YB_YQL_DATA_TYPE_INET:
case YB_YQL_DATA_TYPE_LIST:
case YB_YQL_DATA_TYPE_MAP:
case YB_YQL_DATA_TYPE_SET:
case YB_YQL_DATA_TYPE_UUID:
case YB_YQL_DATA_TYPE_TIMEUUID:
case YB_YQL_DATA_TYPE_TUPLE:
case YB_YQL_DATA_TYPE_TYPEARGS:
case YB_YQL_DATA_TYPE_USER_DEFINED_TYPE:
case YB_YQL_DATA_TYPE_FROZEN:
case YB_YQL_DATA_TYPE_DATE: // Not used for PG storage
case YB_YQL_DATA_TYPE_TIME: // Not used for PG storage
case YB_YQL_DATA_TYPE_JSONB:
case YB_YQL_DATA_TYPE_UINT8:
case YB_YQL_DATA_TYPE_UINT16:
default:
YB_PG_UNSUPPORTED_TYPES_IN_SWITCH:
YB_PG_INVALID_TYPES_IN_SWITCH:
LOG(DFATAL) << "Internal error: unsupported type " << type_entity_->yb_type;
}

Expand Down
39 changes: 5 additions & 34 deletions src/yb/yql/pggate/pg_value.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
//--------------------------------------------------------------------------------------------------

#include "yb/yql/pggate/pg_value.h"
#include "yb/yql/pggate/ybc_pg_typedefs.h"

namespace yb {
namespace pggate {
Expand Down Expand Up @@ -135,23 +136,8 @@ Status PgValueFromPB(const YBCPgTypeEntity *type_entity,
break;
}

case YB_YQL_DATA_TYPE_VARINT:
case YB_YQL_DATA_TYPE_INET:
case YB_YQL_DATA_TYPE_LIST:
case YB_YQL_DATA_TYPE_MAP:
case YB_YQL_DATA_TYPE_SET:
case YB_YQL_DATA_TYPE_UUID:
case YB_YQL_DATA_TYPE_TIMEUUID:
case YB_YQL_DATA_TYPE_TUPLE:
case YB_YQL_DATA_TYPE_TYPEARGS:
case YB_YQL_DATA_TYPE_USER_DEFINED_TYPE:
case YB_YQL_DATA_TYPE_FROZEN:
case YB_YQL_DATA_TYPE_DATE: // Not used for PG storage
case YB_YQL_DATA_TYPE_TIME: // Not used for PG storage
case YB_YQL_DATA_TYPE_JSONB:
case YB_YQL_DATA_TYPE_UINT8:
case YB_YQL_DATA_TYPE_UINT16:
default:
YB_PG_UNSUPPORTED_TYPES_IN_SWITCH:
YB_PG_INVALID_TYPES_IN_SWITCH:
return STATUS_SUBSTITUTE(InternalError, "unsupported type $0", type_entity->yb_type);
}

Expand Down Expand Up @@ -250,23 +236,8 @@ Status PgValueToPB(const YBCPgTypeEntity *type_entity,
ql_value->set_decimal_value(yb_decimal.EncodeToComparable());
break;
}
case YB_YQL_DATA_TYPE_VARINT:
case YB_YQL_DATA_TYPE_INET:
case YB_YQL_DATA_TYPE_LIST:
case YB_YQL_DATA_TYPE_MAP:
case YB_YQL_DATA_TYPE_SET:
case YB_YQL_DATA_TYPE_UUID:
case YB_YQL_DATA_TYPE_TIMEUUID:
case YB_YQL_DATA_TYPE_TUPLE:
case YB_YQL_DATA_TYPE_TYPEARGS:
case YB_YQL_DATA_TYPE_USER_DEFINED_TYPE:
case YB_YQL_DATA_TYPE_FROZEN:
case YB_YQL_DATA_TYPE_DATE: // Not used for PG storage
case YB_YQL_DATA_TYPE_TIME: // Not used for PG storage
case YB_YQL_DATA_TYPE_JSONB:
case YB_YQL_DATA_TYPE_UINT8:
case YB_YQL_DATA_TYPE_UINT16:
default:
YB_PG_UNSUPPORTED_TYPES_IN_SWITCH:
YB_PG_INVALID_TYPES_IN_SWITCH:
return STATUS_SUBSTITUTE(InternalError, "unsupported type $0", type_entity->yb_type);
}
return Status::OK();
Expand Down
27 changes: 27 additions & 0 deletions src/yb/yql/pggate/ybc_pg_typedefs.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,33 @@ typedef enum PgDataType {
YB_YQL_DATA_TYPE_UINT64 = 103
} YBCPgDataType;

// Datatypes that are internally designated to be unsupported.
// (See similar QL_UNSUPPORTED_TYPES_IN_SWITCH.)
#define YB_PG_UNSUPPORTED_TYPES_IN_SWITCH \
case YB_YQL_DATA_TYPE_NOT_SUPPORTED: \
case YB_YQL_DATA_TYPE_UNKNOWN_DATA

// Datatypes that are not used in YSQL.
// (See similar QL_INVALID_TYPES_IN_SWITCH.)
#define YB_PG_INVALID_TYPES_IN_SWITCH \
case YB_YQL_DATA_TYPE_NULL_VALUE_TYPE: \
case YB_YQL_DATA_TYPE_VARINT: \
case YB_YQL_DATA_TYPE_INET: \
case YB_YQL_DATA_TYPE_LIST: \
case YB_YQL_DATA_TYPE_MAP: \
case YB_YQL_DATA_TYPE_SET: \
case YB_YQL_DATA_TYPE_UUID: \
case YB_YQL_DATA_TYPE_TIMEUUID: \
case YB_YQL_DATA_TYPE_TUPLE: \
case YB_YQL_DATA_TYPE_TYPEARGS: \
case YB_YQL_DATA_TYPE_USER_DEFINED_TYPE: \
case YB_YQL_DATA_TYPE_FROZEN: \
case YB_YQL_DATA_TYPE_DATE: \
case YB_YQL_DATA_TYPE_TIME: \
case YB_YQL_DATA_TYPE_JSONB: \
case YB_YQL_DATA_TYPE_UINT8: \
case YB_YQL_DATA_TYPE_UINT16

// Datatype representation:
// Definition of a datatype is divided into two different sections.
// - YBCPgTypeEntity is used to keep static information of a datatype.
Expand Down

0 comments on commit e130615

Please sign in to comment.